[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5550BA77.9060209@zoho.com>
Date: Mon, 11 May 2015 22:19:35 +0800
From: hekuang <hekuang@...o.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>,
He Kuang <hekuang@...wei.com>
CC: masami.hiramatsu.pt@...achi.com, a.p.zijlstra@...llo.nl,
jolsa@...nel.org, mingo@...hat.com, wangnan0@...wei.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] perf probe: Add --range option to show variable
location range
hi, Arnaldo
On 05/11/2015 09:58 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 11, 2015 at 01:10:03PM +0000, He Kuang escreveu:
>> 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.
> Please, when you send a V4 patch like this, state what changed with
> regards to the previous version, as I already applied this patch :-\
>
> - Arnaldo
>
I will check next time.
Thanks.
>> 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.
>>
>> 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 @<generic_perform_write+[324-331]>
>> [INV] struct address_space_operations* a_ops @<generic_perform_write+[55-61,170-176,223-246]>
>> [VAL] (unknown_type) fsdata @<generic_perform_write+[70-307,346-411]>
>> [VAL] loff_t pos @<generic_perform_write+[0-286,286-336,346-411]>
>> [VAL] long int status @<generic_perform_write+[83-342,346-411]>
>> [VAL] long unsigned int bytes @<generic_perform_write+[122-311,320-338,346-403,403-411]>
>> [VAL] struct address_space* mapping @<generic_perform_write+[35-344,346-411]>
>> [VAL] struct iov_iter* i @<generic_perform_write+[0-340,346-411]>
>> [VAL] struct page* page @<generic_perform_write+[70-307,346-411]>
>>
>> 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>
>> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
>> ---
>> tools/perf/builtin-probe.c | 2 +
>> tools/perf/util/dwarf-aux.c | 121 +++++++++++++++++++++++++++++++++++++++++
>> tools/perf/util/dwarf-aux.h | 2 +
>> tools/perf/util/probe-event.h | 1 +
>> tools/perf/util/probe-finder.c | 57 ++++++++++++++-----
>> 5 files changed, 170 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>> index 7fa2c7a..1272559 100644
>> --- a/tools/perf/builtin-probe.c
>> +++ b/tools/perf/builtin-probe.c
>> @@ -372,6 +372,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", &probe_conf.show_ext_vars,
>> "Show external variables too (with --vars only)"),
>> + OPT_BOOLEAN('\0', "range", &probe_conf.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,
>> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
>> index 737c9db..afa0971 100644
>> --- a/tools/perf/util/dwarf-aux.c
>> +++ b/tools/perf/util/dwarf-aux.c
>> @@ -913,3 +913,124 @@ 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
>> + * "@<function_name+[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;
>> + const char *name;
>> +
>> + ret = dwarf_entrypc(sp_die, &entry);
>> + if (ret)
>> + return ret;
>> +
>> + name = dwarf_diename(sp_die);
>> + if (!name)
>> + return -ENOENT;
>> +
>> + count = dwarf_getscopes_die(vr_die, &scopes);
>> +
>> + /* (*SCOPES)[1] is the DIE for the scope containing that scope */
>> + if (count <= 1) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + while ((offset = dwarf_ranges(&scopes[1], offset, &base,
>> + &start, &end)) > 0) {
>> + start -= entry;
>> + end -= entry;
>> +
>> + if (first) {
>> + strbuf_addf(buf, "@<%s+[%lu-%lu",
>> + name, start, end);
>> + first = false;
>> + } else {
>> + strbuf_addf(buf, ",%lu-%lu",
>> + start, end);
>> + }
>> + }
>> +
>> + if (!first)
>> + strbuf_addf(buf, "]>");
>> +
>> +out:
>> + 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
>> + * "@<function_name+[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;
>> + const char *name;
>> +
>> + ret = dwarf_entrypc(sp_die, &entry);
>> + if (ret)
>> + return ret;
>> +
>> + name = dwarf_diename(sp_die);
>> + if (!name)
>> + return -ENOENT;
>> +
>> + 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);
>> + return ret;
>> + }
>> +
>> + /* Location Lists */
>> + start -= entry;
>> + end -= entry;
>> + if (first) {
>> + strbuf_addf(buf, "@<%s+[%lu-%lu",
>> + name, start, end);
>> + first = false;
>> + } else {
>> + strbuf_addf(buf, ",%lu-%lu",
>> + start, end);
>> + }
>> + }
>> +
>> + if (!first)
>> + strbuf_addf(buf, "]>");
>> +
>> + return ret;
>> +}
>> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
>> index 60676fd..c154c0b8 100644
>> --- a/tools/perf/util/dwarf-aux.h
>> +++ b/tools/perf/util/dwarf-aux.h
>> @@ -121,4 +121,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-event.h b/tools/perf/util/probe-event.h
>> index 1e2faa3..537eb32 100644
>> --- a/tools/perf/util/probe-event.h
>> +++ b/tools/perf/util/probe-event.h
>> @@ -9,6 +9,7 @@
>> /* Probe related configurations */
>> struct probe_conf {
>> bool show_ext_vars;
>> + bool show_location_range;
>> bool force_add;
>> bool no_inlines;
>> int max_probes;
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index dcca551..f032329 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -177,7 +177,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 +187,19 @@ 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 (probe_conf.show_location_range &&
>> + (dwarf_tag(vr_die) == DW_TAG_variable)) {
>> + ret2 = -ERANGE;
>> + } 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 +220,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 +230,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,14 +260,14 @@ static_var:
>> }
>>
>> if (!tvar)
>> - return 0;
>> + return ret2;
>>
>> regs = get_arch_regstr(regn);
>> if (!regs) {
>> /* This should be a bug in DWARF or this tool */
>> pr_warning("Mapping for the register number %u "
>> "missing on this architecture.\n", regn);
>> - return -ERANGE;
>> + return -ENOTSUP;
>> }
>>
>> tvar->value = strdup(regs);
>> @@ -269,7 +279,7 @@ static_var:
>> if (tvar->ref == NULL)
>> return -ENOMEM;
>> }
>> - return 0;
>> + return ret2;
>> }
>>
>> #define BYTES_TO_BITS(nb) ((nb) * BITS_PER_LONG / sizeof(long))
>> @@ -1268,13 +1278,34 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>> ret = convert_variable_location(die_mem, af->pf.addr,
>> af->pf.fb_ops, &af->pf.sp_die,
>> NULL);
>> - if (ret == 0) {
>> + if (ret == 0 || ret == -ERANGE) {
>> + int ret2;
>> + bool externs = !af->child;
>> struct strbuf buf;
>>
>> strbuf_init(&buf, 64);
>> - ret = die_get_varname(die_mem, &buf);
>> - pr_debug2("Add new var: %s\n", buf.buf);
>> - if (ret == 0) {
>> +
>> + if (probe_conf.show_location_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 && probe_conf.show_location_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));
>> }
>> --
>> 1.8.5.2
> --
> 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/
>
--
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