Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752926AbbEHOIZ (ORCPT ); Fri, 8 May 2015 10:08:25 -0400 Received: from mail4.hitachi.co.jp ([133.145.228.5]:38452 "EHLO mail4.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752659AbbEHOIY (ORCPT ); Fri, 8 May 2015 10:08:24 -0400 Message-ID: <554CC352.9050302@hitachi.com> Date: Fri, 08 May 2015 23:08:18 +0900 From: Masami Hiramatsu Organization: Hitachi, Ltd., Japan User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: He Kuang , a.p.zijlstra@chello.nl, acme@kernel.org, jolsa@kernel.org, mingo@redhat.com CC: wangnan0@huawei.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] perf probe: Add --range option to show variable location range References: <1431087832-46889-1-git-send-email-hekuang@huawei.com> <1431087832-46889-3-git-send-email-hekuang@huawei.com> In-Reply-To: <1431087832-46889-3-git-send-email-hekuang@huawei.com> Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 14127 Lines: 410 On 2015/05/08 21:23, He Kuang wrote: > It is not easy for users to get the accurate byte offset or the line > number where a local variable can be probed. With '--range' option, > local variables in scope of the probe point are showed with byte offset > range, and can be added according to this range information. Interesting idea :) > > For example, there are some variables in function > generic_perform_write(): > > > 0 ssize_t generic_perform_write(struct file *file, > 1 struct iov_iter *i, loff_t pos) > 2 { > 3 struct address_space *mapping = file->f_mapping; > 4 const struct address_space_operations *a_ops = mapping->a_ops; > ... > 42 status = a_ops->write_begin(file, mapping, pos, bytes, flags, > &page, &fsdata); > 44 if (unlikely(status < 0)) > > But we got failed when we try to probe the variable 'a_ops' at line 42 > or 44. > > $ perf probe --add 'generic_perform_write:42 a_ops' > Failed to find the location of a_ops at this address. > Perhaps, it has been optimized out. Yeah, right. That's why I've introduced --vars option. > > This is because source code do not match assembly, so a variable may not > be available in the sourcecode line where it presents. After this patch, > we can lookup the accurate byte offset range of a variable, 'INV' > indicates that this variable is not valid at the given point, but > available in scope: > > $ perf probe --vars 'generic_perform_write:42' --range > Available variables at generic_perform_write:42 > @ > [INV] ssize_t written [byte offset]: <324-331> > [INV] struct address_space_operations* a_ops [byte offset]: <55-61>,<170-176>,<223-246> > [VAL] (unknown_type) fsdata [byte offset]: <70-307>,<346-411> > [VAL] loff_t pos [byte offset]: <0-286>,<286-336>,<346-411> > [VAL] long int status [byte offset]: <83-342>,<346-411> > [VAL] long unsigned int bytes [byte offset]: <122-311>,<320-338>,<346-403>,<403-411> > [VAL] struct address_space* mapping [byte offset]: <35-344>,<346-411> > [VAL] struct iov_iter* i [byte offset]: <0-340>,<346-411> > [VAL] struct page* page [byte offset]: <70-307>,<346-411> OK, at first, I don't like frequently repeated "[byte offset]", I prefer to show the function name+[offset range], like below :) [INV] ssize_t written @ [INV] struct address_space_operations* a_ops <@generic_perform_write+[55-61,170-176,223-246]> By the way, 'generic_perform_write+170' may be different from given line, is that OK? Thank you, > > Then it is more clearly for us to do the probe with this variable: > > $ perf probe --add 'generic_perform_write+170 a_ops' > Added new event: > probe:generic_perform_write (on generic_perform_write+170 with a_ops) > > Signed-off-by: He Kuang > --- > tools/perf/builtin-probe.c | 6 ++- > tools/perf/util/dwarf-aux.c | 103 +++++++++++++++++++++++++++++++++++++++++ > tools/perf/util/dwarf-aux.h | 2 + > tools/perf/util/probe-finder.c | 69 ++++++++++++++++++++------- > tools/perf/util/probe-finder.h | 3 +- > 5 files changed, 165 insertions(+), 18 deletions(-) > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c > index 19ebe93..1b20004 100644 > --- a/tools/perf/builtin-probe.c > +++ b/tools/perf/builtin-probe.c > @@ -52,6 +52,7 @@ static struct { > bool list_events; > bool force_add; > bool show_ext_vars; > + bool show_location_range; > bool uprobes; > bool quiet; > bool target_used; > @@ -375,6 +376,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused) > "Show accessible variables on PROBEDEF", opt_show_vars), > OPT_BOOLEAN('\0', "externs", ¶ms.show_ext_vars, > "Show external variables too (with --vars only)"), > + OPT_BOOLEAN('\0', "range", ¶ms.show_location_range, > + "Show variables location range in scope (with --vars only)"), > OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name, > "file", "vmlinux pathname"), > OPT_STRING('s', "source", &symbol_conf.source_prefix, > @@ -479,7 +482,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused) > params.filter = strfilter__new(DEFAULT_VAR_FILTER, > NULL); > > - var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN; > + var_flags = params.show_ext_vars << VAR_FLAGS_EXTERN | > + params.show_location_range << VAR_FLAGS_LOCATION_RANGE; > > ret = show_available_vars(params.events, params.nevents, > params.max_probe_points, > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c > index 75d264e..23053c0 100644 > --- a/tools/perf/util/dwarf-aux.c > +++ b/tools/perf/util/dwarf-aux.c > @@ -897,3 +897,106 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf) > return 0; > } > > +/** > + * die_get_var_innermost_scope - Get innermost scope range of given variable DIE > + * @sp_die: a subprogram DIE > + * @vr_die: a variable DIE > + * @buf: a strbuf for variable byte offset range > + * > + * Get the innermost scope range of @vr_die and stores it in @buf as > + * "[byte offset]: ,". > + */ > +static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die, > + struct strbuf *buf) > +{ > + Dwarf_Die *scopes; > + int count; > + size_t offset = 0; > + Dwarf_Addr base; > + Dwarf_Addr start, end; > + Dwarf_Addr entry; > + int ret; > + bool first = true; > + > + ret = dwarf_entrypc(sp_die, &entry); > + if (ret) > + return ret; > + > + count = dwarf_getscopes_die(vr_die, &scopes); > + > + /* (*SCOPES)[1] is the DIE for the scope containing that scope */ > + if (count <= 1) { > + ret = -EINVAL; > + goto end; > + } > + > + while ((offset = dwarf_ranges(&scopes[1], offset, &base, > + &start, &end)) > 0) { > + start -= entry; > + end -= entry; > + > + if (first) { > + strbuf_addf(buf, "[byte offset]: <%ld-%ld>", > + start, end); > + first = false; > + } else { > + strbuf_addf(buf, ",<%ld-%ld>", > + start, end); > + } > + } > +end: > + free(scopes); > + return ret; > +} > + > +/** > + * die_get_var_range - Get byte offset range of given variable DIE > + * @sp_die: a subprogram DIE > + * @vr_die: a variable DIE > + * @buf: a strbuf for type and variable name and byte offset range > + * > + * Get the byte offset range of @vr_die and stores it in @buf as > + * "[byte offset]: ,". > + */ > +int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf) > +{ > + int ret = 0; > + Dwarf_Addr base; > + Dwarf_Addr start, end; > + Dwarf_Addr entry; > + Dwarf_Op *op; > + size_t nops; > + size_t offset = 0; > + Dwarf_Attribute attr; > + bool first = true; > + > + ret = dwarf_entrypc(sp_die, &entry); > + if (ret) > + return ret; > + > + if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL) > + return -EINVAL; > + > + while ((offset = dwarf_getlocations( > + &attr, offset, &base, > + &start, &end, &op, &nops)) > 0) { > + if (start == 0) { > + /* Single Location Descriptions */ > + ret = die_get_var_innermost_scope(sp_die, vr_die, buf); > + } else { > + /* Location Lists */ > + start -= entry; > + end -= entry; > + if (first) { > + strbuf_addf(buf, "[byte offset]: <%ld-%ld>", > + start, end); > + first = false; > + } else { > + strbuf_addf(buf, ",<%ld-%ld>", > + start, end); > + } > + } > + } > + > + return ret; > +} > diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h > index c0d6173..8390cde 100644 > --- a/tools/perf/util/dwarf-aux.h > +++ b/tools/perf/util/dwarf-aux.h > @@ -118,4 +118,6 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf); > > /* Get the name and type of given variable DIE, stored as "type\tname" */ > extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf); > +extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, > + struct strbuf *buf); > #endif > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c > index 6bbdf3f..af0a7ea 100644 > --- a/tools/perf/util/probe-finder.c > +++ b/tools/perf/util/probe-finder.c > @@ -43,6 +43,9 @@ > /* Kprobe tracer basic type is up to u64 */ > #define MAX_BASIC_TYPE_BITS 64 > > +/* Variable location invalid at addr but valid in scope */ > +#define VARIABLE_LOCATION_INVALID_AT_ADDR -10000 > + > /* Dwarf FL wrappers */ > static char *debuginfo_path; /* Currently dummy */ > > @@ -167,7 +170,8 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs) > */ > static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr, > Dwarf_Op *fb_ops, Dwarf_Die *sp_die, > - struct probe_trace_arg *tvar) > + struct probe_trace_arg *tvar, > + unsigned long *var_flags) > { > Dwarf_Attribute attr; > Dwarf_Addr tmp = 0; > @@ -177,7 +181,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr, > Dwarf_Word offs = 0; > bool ref = false; > const char *regs; > - int ret; > + int ret, ret2 = 0; > > if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL) > goto static_var; > @@ -187,9 +191,20 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr, > return -EINVAL; /* Broken DIE ? */ > if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) { > ret = dwarf_entrypc(sp_die, &tmp); > - if (ret || addr != tmp || > - dwarf_tag(vr_die) != DW_TAG_formal_parameter || > - dwarf_highpc(sp_die, &tmp)) > + if (ret) > + return -ENOENT; > + > + if (var_flags && > + test_bit(VAR_FLAGS_LOCATION_RANGE, var_flags) && > + (dwarf_tag(vr_die) == DW_TAG_variable)) { > + ret2 = VARIABLE_LOCATION_INVALID_AT_ADDR; > + } else if (addr != tmp || > + dwarf_tag(vr_die) != DW_TAG_formal_parameter) { > + return -ENOENT; > + } > + > + ret = dwarf_highpc(sp_die, &tmp); > + if (ret) > return -ENOENT; > /* > * This is fuzzed by fentry mcount. We try to find the > @@ -210,7 +225,7 @@ found: > if (op->atom == DW_OP_addr) { > static_var: > if (!tvar) > - return 0; > + return ret2; > /* Static variables on memory (not stack), make @varname */ > ret = strlen(dwarf_diename(vr_die)); > tvar->value = zalloc(ret + 2); > @@ -220,7 +235,7 @@ static_var: > tvar->ref = alloc_trace_arg_ref((long)offs); > if (tvar->ref == NULL) > return -ENOMEM; > - return 0; > + return ret2; > } > > /* If this is based on frame buffer, set the offset */ > @@ -250,7 +265,7 @@ static_var: > } > > if (!tvar) > - return 0; > + return ret2; > > regs = get_arch_regstr(regn); > if (!regs) { > @@ -269,7 +284,7 @@ static_var: > if (tvar->ref == NULL) > return -ENOMEM; > } > - return 0; > + return ret2; > } > > #define BYTES_TO_BITS(nb) ((nb) * BITS_PER_LONG / sizeof(long)) > @@ -516,7 +531,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf) > dwarf_diename(vr_die)); > > ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops, > - &pf->sp_die, pf->tvar); > + &pf->sp_die, pf->tvar, NULL); > if (ret == -ENOENT || ret == -EINVAL) > pr_err("Failed to find the location of %s at this address.\n" > " Perhaps, it has been optimized out.\n", pf->pvar->var); > @@ -1105,7 +1120,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data) > (tag == DW_TAG_variable && vf->vars)) { > if (convert_variable_location(die_mem, vf->pf->addr, > vf->pf->fb_ops, &pf->sp_die, > - NULL) == 0) { > + NULL, NULL) == 0) { > vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem); > if (vf->args[vf->nargs].var == NULL) { > vf->ret = -ENOMEM; > @@ -1254,11 +1269,33 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) > tag == DW_TAG_variable) { > ret = convert_variable_location(die_mem, af->pf.addr, > af->pf.fb_ops, &af->pf.sp_die, > - NULL); > - if (ret == 0) { > - ret = die_get_varname(die_mem, &buf); > - pr_debug2("Add new var: %s\n", buf.buf); > - if (ret == 0) { > + NULL, &af->var_flags); > + if (ret == 0 || ret == VARIABLE_LOCATION_INVALID_AT_ADDR) { > + int ret2; > + bool show_range = test_bit(VAR_FLAGS_LOCATION_RANGE, > + &af->var_flags); > + bool externs = !af->child; > + > + if (show_range) { > + if (!externs) { > + if (ret) > + strbuf_addf(&buf, "[INV]\t"); > + else > + strbuf_addf(&buf, "[VAL]\t"); > + } else > + strbuf_addf(&buf, "[EXT]\t"); > + } > + > + ret2 = die_get_varname(die_mem, &buf); > + > + if (!ret2 && show_range && !externs) { > + strbuf_addf(&buf, "\t"); > + ret2 = die_get_var_range(&af->pf.sp_die, > + die_mem, &buf); > + } > + > + pr_debug("Add new var: %s\n", buf.buf); > + if (ret2 == 0) { > strlist__add(vl->vars, > strbuf_detach(&buf, NULL)); > } > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h > index 95bb79d..4b30a9b 100644 > --- a/tools/perf/util/probe-finder.h > +++ b/tools/perf/util/probe-finder.h > @@ -11,7 +11,8 @@ > #define MAX_PROBE_ARGS 128 > > enum var_flags_bits { > - VAR_FLAGS_EXTERN > + VAR_FLAGS_EXTERN, > + VAR_FLAGS_LOCATION_RANGE > }; > > #define PROBE_ARG_VARS "$vars" > -- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/