Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1553587 > unrolled thread

[PATCH perf/core 1/4] perf-probe: Fix to show correct locations for events on modules

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH perf/core 1/4] perf-probe: Fix to show correct locations for events on modules Masami Hiramatsu <mhiramat@kernel.org> - 2017-01-07 06:30 +0100
    Re: [PATCH perf/core 1/4] perf-probe: Fix to show correct locations  for events on modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-10 14:20 +0100
      Re: [PATCH perf/core 1/4] perf-probe: Fix to show correct locations  for events on modules Masami Hiramatsu <mhiramat@kernel.org> - 2017-01-10 15:20 +0100
      Re: [PATCH perf/core 1/4] perf-probe: Fix to show correct locations  for events on modules Masami Hiramatsu <mhiramat@kernel.org> - 2017-01-11 01:00 +0100

#1553587 — [PATCH perf/core 1/4] perf-probe: Fix to show correct locations for events on modules

Fix to show correct locations for events on modules by
relocating given address. Currently the relocation is
done when we failed to find the address in debuginfo,
but for modules it always makes a mistakes.

E.g. without this fix, events on module seems wrong,
but other cases (kernel and user space) looks good.

  # perf probe -l
    probe:SyS_remap_file_pages (on SyS_remap_file_pages@mm/mmap.c)
    probe:chv_prepare_pll (on intel_plane_atomic_get_property+16@drm/i915/intel_atomic_plane.c in i915)
    probe_perf:alias_lookup (on alias_lookup@util/alias.c in /home/mhiramat/ksrc/linux/tools/perf/perf)

With this fix, all cases are OK now.

  # perf probe -l
    probe:SyS_remap_file_pages (on SyS_remap_file_pages@mm/mmap.c)
    probe:chv_prepare_pll (on chv_prepare_pll@gpu/drm/i915/intel_display.c in i915)
    probe_perf:alias_lookup (on alias_lookup@util/alias.c in /home/mhiramat/ksrc/linux/tools/perf/perf)

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-finder.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index df4debe..0278fe1 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1543,16 +1543,12 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
 	Dwarf_Addr _addr = 0, baseaddr = 0;
 	const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp;
 	int baseline = 0, lineno = 0, ret = 0;
-	bool reloc = false;
 
-retry:
+	/* We always need to relocate the address for aranges */
+	if (debuginfo__get_text_offset(dbg, &baseaddr) == 0)
+		addr += baseaddr;
 	/* Find cu die */
 	if (!dwarf_addrdie(dbg->dbg, (Dwarf_Addr)addr, &cudie)) {
-		if (!reloc && debuginfo__get_text_offset(dbg, &baseaddr) == 0) {
-			addr += baseaddr;
-			reloc = true;
-			goto retry;
-		}
 		pr_warning("Failed to find debug information for address %lx\n",
 			   addr);
 		ret = -EINVAL;

[toc] | [next] | [standalone]

#1555243 — Re: [PATCH perf/core 1/4] perf-probe: Fix to show correct locations for events on modules

Em Sat, Jan 07, 2017 at 02:25:09PM +0900, Masami Hiramatsu escreveu:
> Fix to show correct locations for events on modules by
> relocating given address. Currently the relocation is
> done when we failed to find the address in debuginfo,
> but for modules it always makes a mistakes.

Try to provide precise instructions on how to reproduce, for instance,
here I'm not being able to reproduce:

[root@jouet ~]# perf probe -m i915 chv_prepare_pll
Added new event:
  probe:chv_prepare_pll (on chv_prepare_pll in i915)

You can now use it in all perf tools, such as:

	perf record -e probe:chv_prepare_pll -aR sleep 1

[root@jouet ~]# perf probe -l
  probe:chv_prepare_pll (on chv_prepare_pll in i915)
  probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@intel/e1000e/80003es2lan.c in e1000e)
[root@jouet ~]#

So it doesn't seem to "always make mistakes", what are the precise
conditions to reproduce this problem?

Running with 'perf probe -vv -m i915 chv_prepare_pll' to get more
debugging info:

Failed to get build-id from i915.
Cache open error: -1
Open Debuginfo file: /lib/modules/4.9.0+/kernel/drivers/gpu/drm/i915/i915.ko
Try to find probe point from debuginfo.
Matched function: chv_prepare_pll [6ce853]
found inline addr: 0x883a0
Probe point found: chv_prepare_pll+0
Found 1 probe_trace_events.
Opening /sys/kernel/debug/tracing//kprobe_events write=1
Writing event: p:probe/chv_prepare_pll i915:chv_prepare_pll+0
Added new event:
  probe:chv_prepare_pll (on chv_prepare_pll in i915)

You can now use it in all perf tools, such as:

	perf record -e probe:chv_prepare_pll -aR sleep 1

[root@jouet ~]#

- Arnaldo
 
> E.g. without this fix, events on module seems wrong,
> but other cases (kernel and user space) looks good.
> 
>   # perf probe -l
>     probe:SyS_remap_file_pages (on SyS_remap_file_pages@mm/mmap.c)
>     probe:chv_prepare_pll (on intel_plane_atomic_get_property+16@drm/i915/intel_atomic_plane.c in i915)
>     probe_perf:alias_lookup (on alias_lookup@util/alias.c in /home/mhiramat/ksrc/linux/tools/perf/perf)
> 
> With this fix, all cases are OK now.
> 
>   # perf probe -l
>     probe:SyS_remap_file_pages (on SyS_remap_file_pages@mm/mmap.c)
>     probe:chv_prepare_pll (on chv_prepare_pll@gpu/drm/i915/intel_display.c in i915)
>     probe_perf:alias_lookup (on alias_lookup@util/alias.c in /home/mhiramat/ksrc/linux/tools/perf/perf)
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-finder.c |   10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index df4debe..0278fe1 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1543,16 +1543,12 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
>  	Dwarf_Addr _addr = 0, baseaddr = 0;
>  	const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp;
>  	int baseline = 0, lineno = 0, ret = 0;
> -	bool reloc = false;
>  
> -retry:
> +	/* We always need to relocate the address for aranges */
> +	if (debuginfo__get_text_offset(dbg, &baseaddr) == 0)
> +		addr += baseaddr;
>  	/* Find cu die */
>  	if (!dwarf_addrdie(dbg->dbg, (Dwarf_Addr)addr, &cudie)) {
> -		if (!reloc && debuginfo__get_text_offset(dbg, &baseaddr) == 0) {
> -			addr += baseaddr;
> -			reloc = true;
> -			goto retry;
> -		}
>  		pr_warning("Failed to find debug information for address %lx\n",
>  			   addr);
>  		ret = -EINVAL;

[toc] | [prev] | [next] | [standalone]

#1555436 — Re: [PATCH perf/core 1/4] perf-probe: Fix to show correct locations for events on modules

On Tue, 10 Jan 2017 10:18:35 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Sat, Jan 07, 2017 at 02:25:09PM +0900, Masami Hiramatsu escreveu:
> > Fix to show correct locations for events on modules by
> > relocating given address. Currently the relocation is
> > done when we failed to find the address in debuginfo,
> > but for modules it always makes a mistakes.
> 
> Try to provide precise instructions on how to reproduce, for instance,
> here I'm not being able to reproduce:
> 
> [root@jouet ~]# perf probe -m i915 chv_prepare_pll
> Added new event:
>   probe:chv_prepare_pll (on chv_prepare_pll in i915)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:chv_prepare_pll -aR sleep 1
> 
> [root@jouet ~]# perf probe -l
>   probe:chv_prepare_pll (on chv_prepare_pll in i915)
>   probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@intel/e1000e/80003es2lan.c in e1000e)
> [root@jouet ~]#
> 
> So it doesn't seem to "always make mistakes", what are the precise
> conditions to reproduce this problem?

Hmm, OK, I also found same issue. I'll recheck that...

Thank you,

> 
> Running with 'perf probe -vv -m i915 chv_prepare_pll' to get more
> debugging info:
> 
> Failed to get build-id from i915.
> Cache open error: -1
> Open Debuginfo file: /lib/modules/4.9.0+/kernel/drivers/gpu/drm/i915/i915.ko
> Try to find probe point from debuginfo.
> Matched function: chv_prepare_pll [6ce853]
> found inline addr: 0x883a0
> Probe point found: chv_prepare_pll+0
> Found 1 probe_trace_events.
> Opening /sys/kernel/debug/tracing//kprobe_events write=1
> Writing event: p:probe/chv_prepare_pll i915:chv_prepare_pll+0
> Added new event:
>   probe:chv_prepare_pll (on chv_prepare_pll in i915)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:chv_prepare_pll -aR sleep 1
> 
> [root@jouet ~]#
> 
> - Arnaldo
>  
> > E.g. without this fix, events on module seems wrong,
> > but other cases (kernel and user space) looks good.
> > 
> >   # perf probe -l
> >     probe:SyS_remap_file_pages (on SyS_remap_file_pages@mm/mmap.c)
> >     probe:chv_prepare_pll (on intel_plane_atomic_get_property+16@drm/i915/intel_atomic_plane.c in i915)
> >     probe_perf:alias_lookup (on alias_lookup@util/alias.c in /home/mhiramat/ksrc/linux/tools/perf/perf)
> > 
> > With this fix, all cases are OK now.
> > 
> >   # perf probe -l
> >     probe:SyS_remap_file_pages (on SyS_remap_file_pages@mm/mmap.c)
> >     probe:chv_prepare_pll (on chv_prepare_pll@gpu/drm/i915/intel_display.c in i915)
> >     probe_perf:alias_lookup (on alias_lookup@util/alias.c in /home/mhiramat/ksrc/linux/tools/perf/perf)
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-finder.c |   10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index df4debe..0278fe1 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -1543,16 +1543,12 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
> >  	Dwarf_Addr _addr = 0, baseaddr = 0;
> >  	const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp;
> >  	int baseline = 0, lineno = 0, ret = 0;
> > -	bool reloc = false;
> >  
> > -retry:
> > +	/* We always need to relocate the address for aranges */
> > +	if (debuginfo__get_text_offset(dbg, &baseaddr) == 0)
> > +		addr += baseaddr;
> >  	/* Find cu die */
> >  	if (!dwarf_addrdie(dbg->dbg, (Dwarf_Addr)addr, &cudie)) {
> > -		if (!reloc && debuginfo__get_text_offset(dbg, &baseaddr) == 0) {
> > -			addr += baseaddr;
> > -			reloc = true;
> > -			goto retry;
> > -		}
> >  		pr_warning("Failed to find debug information for address %lx\n",
> >  			   addr);
> >  		ret = -EINVAL;


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1556059 — Re: [PATCH perf/core 1/4] perf-probe: Fix to show correct locations for events on modules

On Tue, 10 Jan 2017 10:18:35 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Sat, Jan 07, 2017 at 02:25:09PM +0900, Masami Hiramatsu escreveu:
> > Fix to show correct locations for events on modules by
> > relocating given address. Currently the relocation is
> > done when we failed to find the address in debuginfo,
> > but for modules it always makes a mistakes.
> 
> Try to provide precise instructions on how to reproduce, for instance,
> here I'm not being able to reproduce:
> 
> [root@jouet ~]# perf probe -m i915 chv_prepare_pll
> Added new event:
>   probe:chv_prepare_pll (on chv_prepare_pll in i915)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:chv_prepare_pll -aR sleep 1
> 
> [root@jouet ~]# perf probe -l
>   probe:chv_prepare_pll (on chv_prepare_pll in i915)
>   probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@intel/e1000e/80003es2lan.c in e1000e)
> [root@jouet ~]#
> 
> So it doesn't seem to "always make mistakes", what are the precise
> conditions to reproduce this problem?

OK, I found mymistakes. chv_prepare_pll in my i915 module is optimized
and have isra.X suffix, so perf ends up with searching it in map.
(That issue is fixed in [3/4] in this series as you may know)


This happens when the module text size is enough big, bigger than
sh_addr, because original code retries with given address + sh_addr
if it failed to find CU DIE at the given address. Any address smaller
than sh_addr always fails and it retries.

On my environment, the sh_addr of ".text" section is 0x10030. Since
i915 is a huge kernel module, we can see this issue as below.

$ grep "[Tt] .*\[i915\]" /proc/kallsyms | sort | head -n1
ffffffffc0270000 t i915_switcheroo_can_switch	[i915]

ffffffffc0270000 + 0x10030 = ffffffffc0280030, so we'll check
symbols cross this boundary.

$ grep "[Tt] .*\[i915\]" /proc/kallsyms | grep -B1 ^ffffffffc028  | head -n 2
ffffffffc027ff80 t haswell_init_clock_gating	[i915]
ffffffffc0280110 t valleyview_init_clock_gating	[i915]

So setup probes on both function and see what happen.

$ sudo ./perf probe -m i915 -a haswell_init_clock_gating -a valleyview_init_clock_gating
Added new events:
  probe:haswell_init_clock_gating (on haswell_init_clock_gating in i915)
  probe:valleyview_init_clock_gating (on valleyview_init_clock_gating in i915)

You can now use it in all perf tools, such as:

	perf record -e probe:valleyview_init_clock_gating -aR sleep 1

$ sudo ./perf probe -l
  probe:haswell_init_clock_gating (on haswell_init_clock_gating@gpu/drm/i915/intel_pm.c in i915)
  probe:valleyview_init_clock_gating (on i915_vga_set_decode:4@gpu/drm/i915/i915_drv.c in i915)

As you can see, haswell_init_clock_gating is correctly shown, but
valleyview_init_clock_gating is not.

With this patch, both events shown correctly.

$ sudo ./perf probe -l
  probe:haswell_init_clock_gating (on haswell_init_clock_gating@gpu/drm/i915/intel_pm.c in i915)
  probe:valleyview_init_clock_gating (on valleyview_init_clock_gating@gpu/drm/i915/intel_pm.c in i915)

I'll update patch description and resend with fix of [4/4].

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web