linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs
@ 2019-10-24  9:12 Masami Hiramatsu
  2019-10-24  9:12 ` [BUGFIX PATCH 1/3] perf/probe: Fix to find range-only function instance Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2019-10-24  9:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Hi,

Here are some bugfixes related to showing available line (--line/-L)
option. I found that gcc generated some subprogram DIE with only
range attribute but no entry_pc or low_pc attributes.
In that case, perf probe failed to show the available lines in that
subprogram (function). To fix that, I introduced some bugfixes to
handle such cases correctly.

Thank you,

---

Masami Hiramatsu (3):
      perf/probe: Fix to find range-only function instance
      perf/probe: Walk function lines in lexical blocks
      perf/probe: Fix to show function entry line as probe-able


 tools/perf/util/dwarf-aux.c |   44 ++++++++++++++++++++++++++++++++++++-------
 tools/perf/util/dwarf-aux.h |    3 +++
 2 files changed, 40 insertions(+), 7 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [BUGFIX PATCH 1/3] perf/probe: Fix to find range-only function instance
  2019-10-24  9:12 [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Masami Hiramatsu
@ 2019-10-24  9:12 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-24  9:12 ` [BUGFIX PATCH 2/3] perf/probe: Walk function lines in lexical blocks Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Masami Hiramatsu @ 2019-10-24  9:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Fix die_is_func_instance() to find range-only function instance.

In some case, a function instance can be made without any low PC
or entry PC, but only with address ranges by optimization.
(e.g. cold text partially in "text.unlikely" section)
To find such function instance, we have to check the range
attribute too.

Fixes: e1ecbbc3fa83 ("perf probe: Fix to handle optimized not-inlined functions")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/dwarf-aux.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index df6cee5c071f..2ec24c3bed44 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -318,10 +318,14 @@ bool die_is_func_def(Dwarf_Die *dw_die)
 bool die_is_func_instance(Dwarf_Die *dw_die)
 {
 	Dwarf_Addr tmp;
+	Dwarf_Attribute attr_mem;
 
 	/* Actually gcc optimizes non-inline as like as inlined */
-	return !dwarf_func_inline(dw_die) && dwarf_entrypc(dw_die, &tmp) == 0;
+	return !dwarf_func_inline(dw_die) &&
+	       (dwarf_entrypc(dw_die, &tmp) == 0 ||
+		dwarf_attr(dw_die, DW_AT_ranges, &attr_mem) != NULL);
 }
+
 /**
  * die_get_data_member_location - Get the data-member offset
  * @mb_die: a DIE of a member of a data structure


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [BUGFIX PATCH 2/3] perf/probe: Walk function lines in lexical blocks
  2019-10-24  9:12 [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Masami Hiramatsu
  2019-10-24  9:12 ` [BUGFIX PATCH 1/3] perf/probe: Fix to find range-only function instance Masami Hiramatsu
@ 2019-10-24  9:12 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-24  9:12 ` [BUGFIX PATCH 3/3] perf/probe: Fix to show function entry line as probe-able Masami Hiramatsu
  2019-10-24 13:43 ` [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Arnaldo Carvalho de Melo
  3 siblings, 1 reply; 11+ messages in thread
From: Masami Hiramatsu @ 2019-10-24  9:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Since some inlined functions are in lexical blocks of given
function, we have to recursively walk through the DIE tree.
Without this fix, perf-probe -L can miss the inlined functions
which is in a lexcical block (like if (..) { func() } case.)

However, even though, to walk the lines in a given function,
we don't need to follow the children DIE of inlined functions
because those do not have any lines in the specified function.

We need to walk though whole trees only if we walk all lines
in a given file, because an inlined function can include
another inlined function in the same file.

Fixes: b0e9cb2802d4 ("perf probe: Fix to search nested inlined functions in CU")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/dwarf-aux.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 2ec24c3bed44..929b7c0567f4 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -678,10 +678,9 @@ static int __die_walk_funclines_cb(Dwarf_Die *in_die, void *data)
 			if (lw->retval != 0)
 				return DIE_FIND_CB_END;
 		}
+		if (!lw->recursive)
+			return DIE_FIND_CB_SIBLING;
 	}
-	if (!lw->recursive)
-		/* Don't need to search recursively */
-		return DIE_FIND_CB_SIBLING;
 
 	if (addr) {
 		fname = dwarf_decl_file(in_die);
@@ -728,6 +727,10 @@ static int __die_walk_culines_cb(Dwarf_Die *sp_die, void *data)
 {
 	struct __line_walk_param *lw = data;
 
+	/*
+	 * Since inlined function can include another inlined function in
+	 * the same file, we need to walk in it recursively.
+	 */
 	lw->retval = __die_walk_funclines(sp_die, true, lw->callback, lw->data);
 	if (lw->retval != 0)
 		return DWARF_CB_ABORT;
@@ -817,8 +820,9 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
 	 */
 	if (rt_die != cu_die)
 		/*
-		 * Don't need walk functions recursively, because nested
-		 * inlined functions don't have lines of the specified DIE.
+		 * Don't need walk inlined functions recursively, because
+		 * inner inlined functions don't have the lines of the
+		 * specified function.
 		 */
 		ret = __die_walk_funclines(rt_die, false, callback, data);
 	else {


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [BUGFIX PATCH 3/3] perf/probe: Fix to show function entry line as probe-able
  2019-10-24  9:12 [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Masami Hiramatsu
  2019-10-24  9:12 ` [BUGFIX PATCH 1/3] perf/probe: Fix to find range-only function instance Masami Hiramatsu
  2019-10-24  9:12 ` [BUGFIX PATCH 2/3] perf/probe: Walk function lines in lexical blocks Masami Hiramatsu
@ 2019-10-24  9:12 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-24 13:43 ` [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Arnaldo Carvalho de Melo
  3 siblings, 1 reply; 11+ messages in thread
From: Masami Hiramatsu @ 2019-10-24  9:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Fix die_walk_lines() to list the function entry line correctly.
Since the dwarf_entrypc() does not return the entry pc if the DIE
has only range attribute, __die_walk_funclines() fails to list
the declaration line (entry line) in that case.

To solve this issue, this introduces die_entrypc() which correctly
returns the entry PC (the first address range) even if the DIE has
only range attribute. With this fix die_walk_lines() shows the
function entry line is able to probe correctly.

Fixes: 4cc9cec636e7 ("perf probe: Introduce lines walker interface")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/dwarf-aux.c |   24 +++++++++++++++++++++++-
 tools/perf/util/dwarf-aux.h |    3 +++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 929b7c0567f4..063f71da6b63 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -307,6 +307,28 @@ bool die_is_func_def(Dwarf_Die *dw_die)
 		dwarf_attr(dw_die, DW_AT_declaration, &attr) == NULL);
 }
 
+/**
+ * die_entrypc - Returns entry PC (the lowest address) of a DIE
+ * @dw_die: a DIE
+ * @addr: where to store entry PC
+ *
+ * Since dwarf_entrypc() does not return entry PC if the DIE has only address
+ * range, we have to use this to retrieve the lowest address from the address
+ * range attribute.
+ */
+int die_entrypc(Dwarf_Die *dw_die, Dwarf_Addr *addr)
+{
+	Dwarf_Addr base, end;
+
+	if (!addr)
+		return -EINVAL;
+
+	if (dwarf_entrypc(dw_die, addr) == 0)
+		return 0;
+
+	return dwarf_ranges(dw_die, 0, &base, addr, &end) < 0 ? -ENOENT : 0;
+}
+
 /**
  * die_is_func_instance - Ensure that this DIE is an instance of a subprogram
  * @dw_die: a DIE
@@ -713,7 +735,7 @@ static int __die_walk_funclines(Dwarf_Die *sp_die, bool recursive,
 	/* Handle function declaration line */
 	fname = dwarf_decl_file(sp_die);
 	if (fname && dwarf_decl_line(sp_die, &lineno) == 0 &&
-	    dwarf_entrypc(sp_die, &addr) == 0) {
+	    die_entrypc(sp_die, &addr) == 0) {
 		lw.retval = callback(fname, lineno, addr, data);
 		if (lw.retval != 0)
 			goto done;
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index f204e5892403..506006e0cf66 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -29,6 +29,9 @@ int cu_walk_functions_at(Dwarf_Die *cu_die, Dwarf_Addr addr,
 /* Get DW_AT_linkage_name (should be NULL for C binary) */
 const char *die_get_linkage_name(Dwarf_Die *dw_die);
 
+/* Get the lowest PC in DIE (including range list) */
+int die_entrypc(Dwarf_Die *dw_die, Dwarf_Addr *addr);
+
 /* Ensure that this DIE is a subprogram and definition (not declaration) */
 bool die_is_func_def(Dwarf_Die *dw_die);
 


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs
  2019-10-24  9:12 [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2019-10-24  9:12 ` [BUGFIX PATCH 3/3] perf/probe: Fix to show function entry line as probe-able Masami Hiramatsu
@ 2019-10-24 13:43 ` Arnaldo Carvalho de Melo
  2019-10-24 15:36   ` Masami Hiramatsu
  3 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-24 13:43 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Em Thu, Oct 24, 2019 at 06:12:27PM +0900, Masami Hiramatsu escreveu:
> Hi,
> 
> Here are some bugfixes related to showing available line (--line/-L)
> option. I found that gcc generated some subprogram DIE with only
> range attribute but no entry_pc or low_pc attributes.
> In that case, perf probe failed to show the available lines in that
> subprogram (function). To fix that, I introduced some bugfixes to
> handle such cases correctly.

Thanks, applied, next time please provide concrete examples for things
that don't work before and gets fixed with your patches, this way we can
more easily reproduce your steps.

- Arnaldo
 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (3):
>       perf/probe: Fix to find range-only function instance
>       perf/probe: Walk function lines in lexical blocks
>       perf/probe: Fix to show function entry line as probe-able
> 
> 
>  tools/perf/util/dwarf-aux.c |   44 ++++++++++++++++++++++++++++++++++++-------
>  tools/perf/util/dwarf-aux.h |    3 +++
>  2 files changed, 40 insertions(+), 7 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs
  2019-10-24 13:43 ` [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Arnaldo Carvalho de Melo
@ 2019-10-24 15:36   ` Masami Hiramatsu
  2019-10-24 20:23     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 11+ messages in thread
From: Masami Hiramatsu @ 2019-10-24 15:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Hi Arnaldo,

On Thu, 24 Oct 2019 10:43:25 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Oct 24, 2019 at 06:12:27PM +0900, Masami Hiramatsu escreveu:
> > Hi,
> > 
> > Here are some bugfixes related to showing available line (--line/-L)
> > option. I found that gcc generated some subprogram DIE with only
> > range attribute but no entry_pc or low_pc attributes.
> > In that case, perf probe failed to show the available lines in that
> > subprogram (function). To fix that, I introduced some bugfixes to
> > handle such cases correctly.
> 
> Thanks, applied, next time please provide concrete examples for things
> that don't work before and gets fixed with your patches, this way we can
> more easily reproduce your steps.

OK, I'll try, but I found that this kind of examples depend on
gcc optimization (like build option, gcc version etc.) So even if you
can not reproduce, don't be disappointed ;)

Anyway, for this series, I found that clear_tasks_mm_cpumask() caused
the (triple) issues.

So, without any patch, I got below error.

$ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask
Specified source line is not found.
  Error: Failed to show lines.

Hmm, there should be something wrong... so I fixed it with [1/3].
(to find appropriate target function, you can use "eu-readelf -w vmlinux"
 and find the subprogram which has no entry_pc nor low_pc but has ranges)

$ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask
<clear_tasks_mm_cpumask@/home/mhiramat/ksrc/mincs/work/linux/linux/kernel/cpu.c:
         void clear_tasks_mm_cpumask(int cpu)
      1  {
      2         struct task_struct *p;
         
                /*
                 * This function is called after the cpu is taken down and marke
                 * offline, so its not like new tasks will ever get this cpu set
                 * their mm mask. -- Peter Zijlstra
                 * Thus, we may use rcu_read_lock() here, instead of grabbing
                 * full-fledged tasklist_lock.
                 */
     11         WARN_ON(cpu_online(cpu));
     12         rcu_read_lock();
     13         for_each_process(p) {
     14                 struct task_struct *t;
         
                        /*
                         * Main thread might exit, but other threads may still h
                         * a valid mm. Find one.
                         */
     20                 t = find_lock_task_mm(p);
     21                 if (!t)
                                continue;
                        cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
                        task_unlock(t);
                }
     26         rcu_read_unlock();
     27  }
         
OK! it looks working... but a bit wired. why line #0, #23 and #24 are not
available?
I investigated it and found a wrong logic and fixed it with [2/3]

$ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask:20
<clear_tasks_mm_cpumask@/home/mhiramat/ksrc/mincs/work/linux/linux/kernel/cpu.c:
     20                 t = find_lock_task_mm(p);
     21                 if (!t)
                                continue;
     23                 cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
     24                 task_unlock(t);
                }
     26         rcu_read_unlock();
     27  }

OK, it found line #23 and #24. And add [3/3]

$ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask
<clear_tasks_mm_cpumask@/home/mhiramat/ksrc/mincs/work/linux/linux/kernel/cpu.c:
      0  void clear_tasks_mm_cpumask(int cpu)
      1  {
      2         struct task_struct *p;

OK, so now it works well :)

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs
  2019-10-24 15:36   ` Masami Hiramatsu
@ 2019-10-24 20:23     ` Arnaldo Carvalho de Melo
  2019-10-24 23:22       ` Masami Hiramatsu
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-24 20:23 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Em Fri, Oct 25, 2019 at 12:36:48AM +0900, Masami Hiramatsu escreveu:
> On Thu, 24 Oct 2019 10:43:25 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> > Em Thu, Oct 24, 2019 at 06:12:27PM +0900, Masami Hiramatsu escreveu:
> > > Here are some bugfixes related to showing available line (--line/-L)
> > > option. I found that gcc generated some subprogram DIE with only
> > > range attribute but no entry_pc or low_pc attributes.
> > > In that case, perf probe failed to show the available lines in that
> > > subprogram (function). To fix that, I introduced some bugfixes to
> > > handle such cases correctly.

> > Thanks, applied, next time please provide concrete examples for things
> > that don't work before and gets fixed with your patches, this way we can
> > more easily reproduce your steps.
 
> OK, I'll try, but I found that this kind of examples depend on
> gcc optimization (like build option, gcc version etc.) So even if you
> can not reproduce, don't be disappointed ;)

Don't worry about that, even if I don't manage to reproduce I'm
convinced that having the set of steps to reproduce the problem, with
actual output from tools, and your step-by-step explanations helps
whoever feels like reviewing your patch, so thanks a lot for doing what
I suggested.
 
> Anyway, for this series, I found that clear_tasks_mm_cpumask() caused
> the (triple) issues.

So, lets try to repro...
 
> So, without any patch, I got below error.
 
> $ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask
> Specified source line is not found.
>   Error: Failed to show lines.

[root@quaco ~]# perf probe clear_tasks_mm_cpumask
Probe point 'clear_tasks_mm_cpumask' not found.
  Error: Failed to add events.
[root@quaco ~]# uname -r
5.2.18-200.fc30.x86_64
[root@quaco ~]# rpm -q kernel-debuginfo
kernel-debuginfo-5.2.15-200.fc30.x86_64
kernel-debuginfo-5.2.17-200.fc30.x86_64
kernel-debuginfo-5.2.18-200.fc30.x86_64
[root@quaco ~]# grep clear_tasks /proc/kallsyms
ffffffffaa0e09a0 T clear_tasks_mm_cpumask
ffffffffaa0e10d3 t clear_tasks_mm_cpumask.cold
[root@quaco ~]#

woah! Seems I'm on the right track :-)

> 
> Hmm, there should be something wrong... so I fixed it with [1/3].
> (to find appropriate target function, you can use "eu-readelf -w vmlinux"

So I don't know where is this vmlinux, lets try to find it:

[root@quaco ~]# perf probe -v clear_tasks_mm_cpumask
probe-definition(0): clear_tasks_mm_cpumask
symbol:clear_tasks_mm_cpumask file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Looking at the vmlinux_path (8 entries long)
Using /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux for symbols
Open Debuginfo file: /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux
Try to find probe point from debuginfo.
Matched function: clear_tasks_mm_cpumask [14d33f6]
clear_tasks_mm_cpumask has no entry PC. Skipped
Symbol clear_tasks_mm_cpumask address found : ffffffff810e09a0
Matched function: clear_tasks_mm_cpumask [14d33f6]
clear_tasks_mm_cpumask has no entry PC. Skipped
Probe point 'clear_tasks_mm_cpumask' not found.
  Error: Failed to add events. Reason: No such file or directory (Code: -2)
[root@quaco ~]#

Ok /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux, then:

[root@quaco ~]# eu-readelf -w /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux | grep clear_tasks_mm_cpumask -B5 -A30
<go grab some coffee>
 [14d3770]      inlined_subroutine   abbrev: 32
               abstract_origin      (ref4) [14d8d01]
               entry_pc             (addr) 0xffffffff810e09b7 <clear_tasks_mm_cpumask+0x17>
               GNU_entry_view       (data2) 7
               ranges               (sec_offset) range list [11e9d0]
               call_file            (data1) cpu.c (1)
               call_line            (data2) 817
               call_column          (data1) 2
               sibling              (ref4) [14d37b0]
 [14d378b]        inlined_subroutine   abbrev: 176
                 abstract_origin      (ref4) [14d8d16]
                 entry_pc             (addr) 0xffffffff810e09b7 <clear_tasks_mm_cpumask+0x17>
                 GNU_entry_view       (data2) 9
                 low_pc               (addr) 0xffffffff810e09b7 <clear_tasks_mm_cpumask+0x17>
                 high_pc              (data8) 0 (0xffffffff810e09b7 <clear_tasks_mm_cpumask+0x17>)
                 call_file            (data1) rcupdate.h (23)
                 call_line            (data2) 591
                 call_column          (data1) 2
 [14d37b0]      inlined_subroutine   abbrev: 53
               abstract_origin      (ref4) [14d8cf6]
               entry_pc             (addr) 0xffffffff810e0a07 <clear_tasks_mm_cpumask+0x67>
               GNU_entry_view       (data2) 2
               ranges               (sec_offset) range list [11ec40]
               call_file            (data1) cpu.c (1)
               call_line            (data2) 831
               call_column          (data1) 2
 [14d37c7]        inlined_subroutine   abbrev: 176
                 abstract_origin      (ref4) [14d8d0c]
                 entry_pc             (addr) 0xffffffff810e0a07 <clear_tasks_mm_cpumask+0x67>
                 GNU_entry_view       (data2) 8
                 low_pc               (addr) 0xffffffff810e0a07 <clear_tasks_mm_cpumask+0x67>
                 high_pc              (data8) 0 (0xffffffff810e0a07 <clear_tasks_mm_cpumask+0x67>)
                 call_file            (data1) rcupdate.h (23)
                 call_line            (data2) 646
                 call_column          (data1


>  and find the subprogram which has no entry_pc nor low_pc but has ranges)
> 
> $ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask
> <clear_tasks_mm_cpumask@/home/mhiramat/ksrc/mincs/work/linux/linux/kernel/cpu.c:
>          void clear_tasks_mm_cpumask(int cpu)
>       1  {
>       2         struct task_struct *p;
>          
>                 /*
>                  * This function is called after the cpu is taken down and marke
>                  * offline, so its not like new tasks will ever get this cpu set
>                  * their mm mask. -- Peter Zijlstra
>                  * Thus, we may use rcu_read_lock() here, instead of grabbing
>                  * full-fledged tasklist_lock.
>                  */
>      11         WARN_ON(cpu_online(cpu));
>      12         rcu_read_lock();
>      13         for_each_process(p) {
>      14                 struct task_struct *t;
>          
>                         /*
>                          * Main thread might exit, but other threads may still h
>                          * a valid mm. Find one.
>                          */
>      20                 t = find_lock_task_mm(p);
>      21                 if (!t)
>                                 continue;
>                         cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
>                         task_unlock(t);
>                 }
>      26         rcu_read_unlock();
>      27  }
>          
> OK! it looks working... but a bit wired. why line #0, #23 and #24 are not
> available?
> I investigated it and found a wrong logic and fixed it with [2/3]
> 
> $ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask:20
> <clear_tasks_mm_cpumask@/home/mhiramat/ksrc/mincs/work/linux/linux/kernel/cpu.c:
>      20                 t = find_lock_task_mm(p);
>      21                 if (!t)
>                                 continue;
>      23                 cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
>      24                 task_unlock(t);
>                 }
>      26         rcu_read_unlock();
>      27  }
> 
> OK, it found line #23 and #24. And add [3/3]
> 
> $ tools/perf/perf probe -k ../build-x86_64/vmlinux -L clear_tasks_mm_cpumask
> <clear_tasks_mm_cpumask@/home/mhiramat/ksrc/mincs/work/linux/linux/kernel/cpu.c:
>       0  void clear_tasks_mm_cpumask(int cpu)
>       1  {
>       2         struct task_struct *p;
> 
> OK, so now it works well :)

Ok, so, the above output is with your patches, as well as the following,
which, for -L, matches your results:

[root@quaco ~]# perf probe -L clear_tasks_mm_cpumask
<clear_tasks_mm_cpumask@/usr/src/debug/kernel-5.2.fc30/linux-5.2.18-200.fc30.x86_64/kernel/cpu.c:0>
      0  void clear_tasks_mm_cpumask(int cpu)
      1  {
      2         struct task_struct *p;

                /*
                 * This function is called after the cpu is taken down and marked
                 * offline, so its not like new tasks will ever get this cpu set in
                 * their mm mask. -- Peter Zijlstra
                 * Thus, we may use rcu_read_lock() here, instead of grabbing
                 * full-fledged tasklist_lock.
                 */
     11         WARN_ON(cpu_online(cpu));
     12         rcu_read_lock();
     13         for_each_process(p) {
     14                 struct task_struct *t;

                        /*
                         * Main thread might exit, but other threads may still have
                         * a valid mm. Find one.
                         */
     20                 t = find_lock_task_mm(p);
     21                 if (!t)
                                continue;
     23                 cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
     24                 task_unlock(t);
                }
     26         rcu_read_unlock();
     27  }

         /* Take this CPU down. */
         static int take_cpu_down(void *_param)

[root@quaco ~]# perf probe clear_tasks_mm_cpumask
Probe point 'clear_tasks_mm_cpumask' not found.
  Error: Failed to add events.
[root@quaco ~]#

But why can't I probe this thing again?

[root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
Probe point 'clear_tasks_mm_cpumask' not found.
  Error: Failed to add events.
[root@quaco ~]# perf probe clear_tasks_mm_cpumask:1
Failed to get entry address of clear_tasks_mm_cpumask
  Error: Failed to add events.
[root@quaco ~]# perf probe clear_tasks_mm_cpumask:2
Failed to get entry address of clear_tasks_mm_cpumask
  Error: Failed to add events.
[root@quaco ~]# perf probe clear_tasks_mm_cpumask:11
Failed to get entry address of clear_tasks_mm_cpumask
  Error: Failed to add events.
[root@quaco ~]# perf probe clear_tasks_mm_cpumask:12
Failed to get entry address of clear_tasks_mm_cpumask
  Error: Failed to add events.
[root@quaco ~]# perf probe clear_tasks_mm_cpumask:13
Failed to get entry address of clear_tasks_mm_cpumask
  Error: Failed to add events.
root@quaco ~]#

[root@quaco ~]# rpm -qf /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux
kernel-debuginfo-5.2.18-200.fc30.x86_64
[root@quaco ~]#

[root@quaco ~]# readelf -wi /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux | grep "DW_AT_producer.*GNU C" -m1 -A6
    <2e>   DW_AT_producer    : (indirect string, offset: 0xf1c6): GNU C89 9.2.1 20190827 (Red Hat 9.2.1-1) -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel -mindirect-branch=thunk-extern -mindirect-branch-register -mrecord-mcount -mfentry -march=x86-64 -g -O2 -std=gnu90 -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -falign-jumps=1 -falign-loops=1 -fno-asynchronous-unwind-tables -fno-jump-tables -fno-delete-null-pointer-checks -fstack-protector-strong -fvar-tracking-assignments -fno-strict-overflow -fno-merge-all-constants -fmerge-constants -fstack-check=no -fconserve-stack --param allow-store-data-races=0
    <32>   DW_AT_language    : 1	(ANSI C)
    <33>   DW_AT_name        : (indirect string, offset: 0xdfe59): arch/x86/kernel/head64.c
    <37>   DW_AT_comp_dir    : (indirect string, offset: 0x4d33f): /usr/src/debug/kernel-5.2.fc30/linux-5.2.18-200.fc30.x86_64
    <3b>   DW_AT_ranges      : 0x5f0
    <3f>   DW_AT_low_pc      : 0x0
    <47>   DW_AT_stmt_list   : 0x165
[root@quaco ~]#

- Arnaldo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs
  2019-10-24 20:23     ` Arnaldo Carvalho de Melo
@ 2019-10-24 23:22       ` Masami Hiramatsu
  0 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2019-10-24 23:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

On Thu, 24 Oct 2019 17:23:27 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:


> But why can't I probe this thing again?
> 
> [root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
> Probe point 'clear_tasks_mm_cpumask' not found.
>   Error: Failed to add events.
> [root@quaco ~]# perf probe clear_tasks_mm_cpumask:1
> Failed to get entry address of clear_tasks_mm_cpumask
>   Error: Failed to add events.
> [root@quaco ~]# perf probe clear_tasks_mm_cpumask:2
> Failed to get entry address of clear_tasks_mm_cpumask
>   Error: Failed to add events.
> [root@quaco ~]# perf probe clear_tasks_mm_cpumask:11
> Failed to get entry address of clear_tasks_mm_cpumask
>   Error: Failed to add events.
> [root@quaco ~]# perf probe clear_tasks_mm_cpumask:12
> Failed to get entry address of clear_tasks_mm_cpumask
>   Error: Failed to add events.
> [root@quaco ~]# perf probe clear_tasks_mm_cpumask:13
> Failed to get entry address of clear_tasks_mm_cpumask
>   Error: Failed to add events.
> root@quaco ~]#

Oops, it's a verification error in convert_to_trace_point().
I'll fix it too!

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tip: perf/core] perf probe: Walk function lines in lexical blocks
  2019-10-24  9:12 ` [BUGFIX PATCH 2/3] perf/probe: Walk function lines in lexical blocks Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Jiri Olsa, Namhyung Kim,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     acb6a7047ac2146b723fef69ee1ab6b7143546bf
Gitweb:        https://git.kernel.org/tip/acb6a7047ac2146b723fef69ee1ab6b7143546bf
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Thu, 24 Oct 2019 18:12:45 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Walk function lines in lexical blocks

Since some inlined functions are in lexical blocks of given function, we
have to recursively walk through the DIE tree.  Without this fix,
perf-probe -L can miss the inlined functions which is in a lexical block
(like if (..) { func() } case.)

However, even though, to walk the lines in a given function, we don't
need to follow the children DIE of inlined functions because those do
not have any lines in the specified function.

We need to walk though whole trees only if we walk all lines in a given
file, because an inlined function can include another inlined function
in the same file.

Fixes: b0e9cb2802d4 ("perf probe: Fix to search nested inlined functions in CU")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157190836514.1859.15996864849678136353.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 2ec24c3..929b7c0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -678,10 +678,9 @@ static int __die_walk_funclines_cb(Dwarf_Die *in_die, void *data)
 			if (lw->retval != 0)
 				return DIE_FIND_CB_END;
 		}
+		if (!lw->recursive)
+			return DIE_FIND_CB_SIBLING;
 	}
-	if (!lw->recursive)
-		/* Don't need to search recursively */
-		return DIE_FIND_CB_SIBLING;
 
 	if (addr) {
 		fname = dwarf_decl_file(in_die);
@@ -728,6 +727,10 @@ static int __die_walk_culines_cb(Dwarf_Die *sp_die, void *data)
 {
 	struct __line_walk_param *lw = data;
 
+	/*
+	 * Since inlined function can include another inlined function in
+	 * the same file, we need to walk in it recursively.
+	 */
 	lw->retval = __die_walk_funclines(sp_die, true, lw->callback, lw->data);
 	if (lw->retval != 0)
 		return DWARF_CB_ABORT;
@@ -817,8 +820,9 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
 	 */
 	if (rt_die != cu_die)
 		/*
-		 * Don't need walk functions recursively, because nested
-		 * inlined functions don't have lines of the specified DIE.
+		 * Don't need walk inlined functions recursively, because
+		 * inner inlined functions don't have the lines of the
+		 * specified function.
 		 */
 		ret = __die_walk_funclines(rt_die, false, callback, data);
 	else {

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [tip: perf/core] perf probe: Fix to show function entry line as probe-able
  2019-10-24  9:12 ` [BUGFIX PATCH 3/3] perf/probe: Fix to show function entry line as probe-able Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Jiri Olsa, Namhyung Kim,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     91e2f539eeda26ab00bd03fae8dc434c128c85ed
Gitweb:        https://git.kernel.org/tip/91e2f539eeda26ab00bd03fae8dc434c128c85ed
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Thu, 24 Oct 2019 18:12:54 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix to show function entry line as probe-able

Fix die_walk_lines() to list the function entry line correctly.  Since
the dwarf_entrypc() does not return the entry pc if the DIE has only
range attribute, __die_walk_funclines() fails to list the declaration
line (entry line) in that case.

To solve this issue, this introduces die_entrypc() which correctly
returns the entry PC (the first address range) even if the DIE has only
range attribute. With this fix die_walk_lines() shows the function entry
line is able to probe correctly.

Fixes: 4cc9cec636e7 ("perf probe: Introduce lines walker interface")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157190837419.1859.4619125803596816752.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 24 +++++++++++++++++++++++-
 tools/perf/util/dwarf-aux.h |  3 +++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 929b7c0..063f71d 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -308,6 +308,28 @@ bool die_is_func_def(Dwarf_Die *dw_die)
 }
 
 /**
+ * die_entrypc - Returns entry PC (the lowest address) of a DIE
+ * @dw_die: a DIE
+ * @addr: where to store entry PC
+ *
+ * Since dwarf_entrypc() does not return entry PC if the DIE has only address
+ * range, we have to use this to retrieve the lowest address from the address
+ * range attribute.
+ */
+int die_entrypc(Dwarf_Die *dw_die, Dwarf_Addr *addr)
+{
+	Dwarf_Addr base, end;
+
+	if (!addr)
+		return -EINVAL;
+
+	if (dwarf_entrypc(dw_die, addr) == 0)
+		return 0;
+
+	return dwarf_ranges(dw_die, 0, &base, addr, &end) < 0 ? -ENOENT : 0;
+}
+
+/**
  * die_is_func_instance - Ensure that this DIE is an instance of a subprogram
  * @dw_die: a DIE
  *
@@ -713,7 +735,7 @@ static int __die_walk_funclines(Dwarf_Die *sp_die, bool recursive,
 	/* Handle function declaration line */
 	fname = dwarf_decl_file(sp_die);
 	if (fname && dwarf_decl_line(sp_die, &lineno) == 0 &&
-	    dwarf_entrypc(sp_die, &addr) == 0) {
+	    die_entrypc(sp_die, &addr) == 0) {
 		lw.retval = callback(fname, lineno, addr, data);
 		if (lw.retval != 0)
 			goto done;
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index f204e58..506006e 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -29,6 +29,9 @@ int cu_walk_functions_at(Dwarf_Die *cu_die, Dwarf_Addr addr,
 /* Get DW_AT_linkage_name (should be NULL for C binary) */
 const char *die_get_linkage_name(Dwarf_Die *dw_die);
 
+/* Get the lowest PC in DIE (including range list) */
+int die_entrypc(Dwarf_Die *dw_die, Dwarf_Addr *addr);
+
 /* Ensure that this DIE is a subprogram and definition (not declaration) */
 bool die_is_func_def(Dwarf_Die *dw_die);
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [tip: perf/core] perf probe: Fix to find range-only function instance
  2019-10-24  9:12 ` [BUGFIX PATCH 1/3] perf/probe: Fix to find range-only function instance Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Jiri Olsa, Namhyung Kim,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     b77afa1f810f37bd8a36cb1318178dfe2d7af6b6
Gitweb:        https://git.kernel.org/tip/b77afa1f810f37bd8a36cb1318178dfe2d7af6b6
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Thu, 24 Oct 2019 18:12:36 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:05 -03:00

perf probe: Fix to find range-only function instance

Fix die_is_func_instance() to find range-only function instance.

In some case, a function instance can be made without any low PC or
entry PC, but only with address ranges by optimization.  (e.g. cold text
partially in "text.unlikely" section) To find such function instance, we
have to check the range attribute too.

Fixes: e1ecbbc3fa83 ("perf probe: Fix to handle optimized not-inlined functions")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157190835669.1859.8368628035930950596.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index df6cee5..2ec24c3 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -318,10 +318,14 @@ bool die_is_func_def(Dwarf_Die *dw_die)
 bool die_is_func_instance(Dwarf_Die *dw_die)
 {
 	Dwarf_Addr tmp;
+	Dwarf_Attribute attr_mem;
 
 	/* Actually gcc optimizes non-inline as like as inlined */
-	return !dwarf_func_inline(dw_die) && dwarf_entrypc(dw_die, &tmp) == 0;
+	return !dwarf_func_inline(dw_die) &&
+	       (dwarf_entrypc(dw_die, &tmp) == 0 ||
+		dwarf_attr(dw_die, DW_AT_ranges, &attr_mem) != NULL);
 }
+
 /**
  * die_get_data_member_location - Get the data-member offset
  * @mb_die: a DIE of a member of a data structure

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-11-12 11:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24  9:12 [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Masami Hiramatsu
2019-10-24  9:12 ` [BUGFIX PATCH 1/3] perf/probe: Fix to find range-only function instance Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-24  9:12 ` [BUGFIX PATCH 2/3] perf/probe: Walk function lines in lexical blocks Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-24  9:12 ` [BUGFIX PATCH 3/3] perf/probe: Fix to show function entry line as probe-able Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-24 13:43 ` [BUGFIX PATCH 0/3] perf/probe: Fix available line bugs Arnaldo Carvalho de Melo
2019-10-24 15:36   ` Masami Hiramatsu
2019-10-24 20:23     ` Arnaldo Carvalho de Melo
2019-10-24 23:22       ` Masami Hiramatsu

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