public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Cc: acme@kernel.org, linux-kernel@vger.kernel.org, srikar@linux.vnet.ibm.com
Subject: Re: [PATCH] perf probe: Ignore tail calls to probed functions
Date: Fri, 01 May 2015 08:41:57 +0900	[thread overview]
Message-ID: <5542BDC5.90609@hitachi.com> (raw)
In-Reply-To: <20150430145834.GA5694@naverao1-tp.in.ibm.com>

On 2015/04/30 23:58, Naveen N. Rao wrote:
> [CC'ing Srikar]
> 
> On 2015/04/30 10:06PM, Masami Hiramatsu wrote:
>> On 2015/04/30 20:42, Naveen N. Rao wrote:
>>> perf probe currently errors out if there are any tail calls to probed
>>> functions:
>>>
>>> [root@rhel71be]# perf probe do_fork
>>> Failed to find probe point in any functions.
>>>   Error: Failed to add events.
>>>
>>> Fix this by teaching perf to ignore tail calls.
>>
>> Hmm, why just ignore it, can't we use it?
>> Also, could you show us how the result will be changed with this?
> 
> Without patch:
> 
> [root@rhel71be perf]# ./perf probe -v do_fork
> probe-definition(0): do_fork symbol:do_fork file:(null) line:0 offset:0 
> return:0 lazy:(null)
> 0 arguments
> Looking at the vmlinux_path (7 entries long)
> symsrc__init: build id mismatch for /boot/vmlinux.
> Using /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux for 
> symbols
> Open Debuginfo file: 
> /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux
> Try to find probe point from debuginfo.
> found inline addr: 0xc0000000000bb9b0
> Probe point found: do_fork+0
> found inline addr: 0xc0000000000bbe20
> Probe point found: kernel_thread+48
> found inline addr: 0xc0000000000bbe5c
> Probe point found: sys_fork+28
> found inline addr: 0xc0000000000bbfac
> Probe point found: sys_vfork+44
> found inline addr: 0xc0000000000bc27c
> Failed to find probe point in any functions.
> An error occurred in debuginfo analysis (-2).
>   Error: Failed to add events. Reason: No such file or directory (Code: 
>   -2)
> 
> With patch:
> 
> [root@rhel71be perf]# ./perf probe -v do_fork
> probe-definition(0): do_fork symbol:do_fork file:(null) line:0 offset:0 
> return:0 lazy:(null)
> 0 arguments
> Looking at the vmlinux_path (7 entries long)
> symsrc__init: build id mismatch for /boot/vmlinux.
> Using /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux for 
> symbols
> Open Debuginfo file: 
> /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux
> Try to find probe point from debuginfo.
> found inline addr: 0xc0000000000bb9b0
> Probe point found: do_fork+0
> found inline addr: 0xc0000000000bbe20
> Probe point found: kernel_thread+48
> found inline addr: 0xc0000000000bbe5c
> Probe point found: sys_fork+28
> found inline addr: 0xc0000000000bbfac
> Probe point found: sys_vfork+44
> found inline addr: 0xc0000000000bc27c
> Ignoring tail call from SyS_clone
> Found 4 probe_trace_events.
> Opening /sys/kernel/debug/tracing/kprobe_events write=1
> No kprobe blacklist support, ignored
> Added new events:
> Writing event: p:probe/do_fork _text+768432
> Failed to write event: Invalid argument
>   Error: Failed to add events. Reason: Invalid argument (Code: -22)
> 
> [Ignore the error about failure to write event - this kernel is missing 
> a patch to resolve _text properly]

OK, so this seems a special case. Other points are still supported.

> 
> The reason to ignore tail calls is that the address does not belong to 
> any function frame. In the example above, the address in SyS_clone is 
> 0xc0000000000bc27c, but looking at the debug-info:
> 
>  <1><830081>: Abbrev Number: 133 (DW_TAG_subprogram)
>     <830083>   DW_AT_external    : 1	
>     <830083>   DW_AT_name        : (indirect string, offset: 0x3cea3): SyS_clone	
>     <830087>   DW_AT_decl_file   : 7	
>     <830088>   DW_AT_decl_line   : 1689	
>     <83008a>   DW_AT_prototyped  : 1	
>     <83008a>   DW_AT_type        : <0x8110eb>	
>     <83008e>   DW_AT_low_pc      : 0xc0000000000bc270	
>     <830096>   DW_AT_high_pc     : 0xc	
>     <83009e>   DW_AT_frame_base  : 1 byte block: 9c 	(DW_OP_call_frame_cfa)
>     <8300a0>   DW_AT_GNU_all_call_sites: 1	
>     <8300a0>   DW_AT_sibling     : <0x830178>	
> <snip>
>  <3><830147>: Abbrev Number: 125 (DW_TAG_GNU_call_site)
>     <830148>   DW_AT_low_pc      : 0xc0000000000bc27c	
>     <830150>   DW_AT_GNU_tail_call: 1	
>     <830150>   DW_AT_abstract_origin: <0x82e7e1>
> 
> The frame ends at 0xc0000000000bc27c. I suppose this is why this 
> particular call is a "tail" call. FWIW, systemtap seems to ignore these 
> as well and requires users to explicitly place probes at these call 
> sites if necessary. I print out the caller so that users know.

Ah, OK. That is out of the function frame, thus we can't put probe on it.
Hmm, we'd better wait for improvement of tail call support in dwarf...
Anyway, at this point, I'm OK for ignoring the probe point.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thank you!


> 
>>
>> Thank you,
> 
> Thanks for the review!
> - Naveen
> 
>>
>>>
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>> ---
>>> I'm seeing this with RHEL7 kernels on ppc64. The specific example in this case is do_fork:
>>>
>>>    <1><82e7e1>: Abbrev Number: 118 (DW_TAG_subprogram)
>>>       <82e7e2>   DW_AT_external    : 1
>>>       <82e7e2>   DW_AT_name        : (indirect string, offset: 0x3dfe0): do_fork
>>>       <82e7e6>   DW_AT_decl_file   : 7
>>>       <82e7e7>   DW_AT_decl_line   : 1583
>>>       <82e7e9>   DW_AT_prototyped  : 1
>>>       <82e7e9>   DW_AT_type        : <0x8110eb>
>>>       <82e7ed>   DW_AT_inline      : 1    (inlined)
>>>       <82e7ee>   DW_AT_sibling     : <0x82e878>
>>>
>>> <snip>
>>>
>>>    <1><830081>: Abbrev Number: 133 (DW_TAG_subprogram)
>>>       <830083>   DW_AT_external    : 1
>>>       <830083>   DW_AT_name        : (indirect string, offset: 0x3cea3): SyS_clone
>>>       <830087>   DW_AT_decl_file   : 7
>>>       <830088>   DW_AT_decl_line   : 1689
>>>       <83008a>   DW_AT_prototyped  : 1
>>>       <83008a>   DW_AT_type        : <0x8110eb>
>>>       <83008e>   DW_AT_low_pc      : 0xc0000000000bc270
>>>       <830096>   DW_AT_high_pc     : 0xc
>>>       <83009e>   DW_AT_frame_base  : 1 byte block: 9c (DW_OP_call_frame_cfa)
>>>       <8300a0>   DW_AT_GNU_all_call_sites: 1
>>>       <8300a0>   DW_AT_sibling     : <0x830178>
>>>
>>> <snip>
>>>
>>>    <3><830147>: Abbrev Number: 125 (DW_TAG_GNU_call_site)
>>>       <830148>   DW_AT_low_pc      : 0xc0000000000bc27c
>>>       <830150>   DW_AT_GNU_tail_call: 1
>>>       <830150>   DW_AT_abstract_origin: <0x82e7e1>
>>>
>>>
>>> - Naveen
>>>
>>>  tools/perf/util/dwarf-aux.c    | 37 +++++++++++++++++++++++++++++++++++++
>>>  tools/perf/util/dwarf-aux.h    |  4 ++++
>>>  tools/perf/util/probe-finder.c | 12 +++++++++---
>>>  3 files changed, 50 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
>>> index c34e024..851a76f 100644
>>> --- a/tools/perf/util/dwarf-aux.c
>>> +++ b/tools/perf/util/dwarf-aux.c
>>> @@ -417,6 +417,43 @@ struct __addr_die_search_param {
>>>  	Dwarf_Die	*die_mem;
>>>  };
>>>  
>>> +static int __die_search_func_tail_cb(Dwarf_Die *fn_die, void *data)
>>> +{
>>> +	struct __addr_die_search_param *ad = data;
>>> +	Dwarf_Addr addr = 0;
>>> +
>>> +	if (dwarf_tag(fn_die) == DW_TAG_subprogram &&
>>> +	    !dwarf_highpc(fn_die, &addr) &&
>>> +	    addr == ad->addr) {
>>> +		memcpy(ad->die_mem, fn_die, sizeof(Dwarf_Die));
>>> +		return DWARF_CB_ABORT;
>>> +	}
>>> +	return DWARF_CB_OK;
>>> +}
>>> +
>>> +/**
>>> + * die_find_tailfunc - Search for a non-inlined function with tail call at
>>> + * given address
>>> + * @cu_die: a CU DIE which including @addr
>>> + * @addr: target address
>>> + * @die_mem: a buffer for result DIE
>>> + *
>>> + * Search for a non-inlined function DIE with tail call at @addr. Stores the
>>> + * DIE to @die_mem and returns it if found. Returns NULL if failed.
>>> + */
>>> +Dwarf_Die *die_find_tailfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
>>> +				    Dwarf_Die *die_mem)
>>> +{
>>> +	struct __addr_die_search_param ad;
>>> +	ad.addr = addr;
>>> +	ad.die_mem = die_mem;
>>> +	/* dwarf_getscopes can't find subprogram. */
>>> +	if (!dwarf_getfuncs(cu_die, __die_search_func_tail_cb, &ad, 0))
>>> +		return NULL;
>>> +	else
>>> +		return die_mem;
>>> +}
>>> +
>>>  /* die_find callback for non-inlined function search */
>>>  static int __die_search_func_cb(Dwarf_Die *fn_die, void *data)
>>>  {
>>> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
>>> index af7dbcd..c9278ed 100644
>>> --- a/tools/perf/util/dwarf-aux.h
>>> +++ b/tools/perf/util/dwarf-aux.h
>>> @@ -82,6 +82,10 @@ extern Dwarf_Die *die_find_child(Dwarf_Die *rt_die,
>>>  extern Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
>>>  				    Dwarf_Die *die_mem);
>>>  
>>> +/* Search a non-inlined function with tail call at given address */
>>> +Dwarf_Die *die_find_tailfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
>>> +				    Dwarf_Die *die_mem);
>>> +
>>>  /* Search the top inlined function including given address */
>>>  extern Dwarf_Die *die_find_top_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
>>>  					  Dwarf_Die *die_mem);
>>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>>> index b5bf9d5..4cb461e 100644
>>> --- a/tools/perf/util/probe-finder.c
>>> +++ b/tools/perf/util/probe-finder.c
>>> @@ -660,9 +660,15 @@ static int call_probe_finder(Dwarf_Die *sc_die, struct probe_finder *pf)
>>>  	/* If not a real subprogram, find a real one */
>>>  	if (!die_is_func_def(sc_die)) {
>>>  		if (!die_find_realfunc(&pf->cu_die, pf->addr, &pf->sp_die)) {
>>> -			pr_warning("Failed to find probe point in any "
>>> -				   "functions.\n");
>>> -			return -ENOENT;
>>> +			if (die_find_tailfunc(&pf->cu_die, pf->addr, &pf->sp_die)) {
>>> +				pr_warning("Ignoring tail call from %s\n",
>>> +						dwarf_diename(&pf->sp_die));
>>> +				return 0;
>>> +			} else {
>>> +				pr_warning("Failed to find probe point in any "
>>> +					   "functions.\n");
>>> +				return -ENOENT;
>>> +			}
>>>  		}
>>>  	} else
>>>  		memcpy(&pf->sp_die, sc_die, sizeof(Dwarf_Die));
>>>
>>
>>
>> -- 
>> Masami HIRAMATSU
>> Linux Technology Research Center, System Productivity Research Dept.
>> Center for Technology Innovation - Systems Engineering
>> Hitachi, Ltd., Research & Development Group
>> E-mail: masami.hiramatsu.pt@hitachi.com
>>
> 
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

  reply	other threads:[~2015-04-30 23:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30 11:42 [PATCH] perf probe: Ignore tail calls to probed functions Naveen N. Rao
2015-04-30 13:06 ` Masami Hiramatsu
2015-04-30 14:58   ` Naveen N. Rao
2015-04-30 23:41     ` Masami Hiramatsu [this message]
2015-05-12 13:40       ` Arnaldo Carvalho de Melo
2015-05-15  6:46 ` [tip:perf/core] " tip-bot for Naveen N. Rao

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=5542BDC5.90609@hitachi.com \
    --to=masami.hiramatsu.pt@hitachi.com \
    --cc=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=naveen.n.rao@linux.vnet.ibm.com \
    --cc=srikar@linux.vnet.ibm.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