[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <554CC352.9050302@hitachi.com>
Date: Fri, 08 May 2015 23:08:18 +0900
From: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To: He Kuang <hekuang@...wei.com>, a.p.zijlstra@...llo.nl,
acme@...nel.org, jolsa@...nel.org, mingo@...hat.com
CC: wangnan0@...wei.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] perf probe: Add --range option to show variable location
range
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():
>
> <generic_perform_write@...filemap.c:0>
> 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
> @<generic_perform_write+141>
> [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 @<generic_perform_write+[324-331]>
[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 <hekuang@...wei.com>
> ---
> 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]: <NN-NN>,<NN-NN>".
> + */
> +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]: <NN-NN>,<NN-NN>".
> + */
> +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@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists