[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <554ECEB9.2040209@hitachi.com>
Date: Sun, 10 May 2015 12:21:29 +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 v2 2/3] perf probe: Add --range option to show variable
location range
On 2015/05/09 18:55, 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.
>
> 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]>
>
Thanks, this looks easier to understand :)
[...]
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index dcca551..30a1a1b 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
Hmm, could you use -ERANGE instead of this?
Other part is OK for me.
Thank you!
> +
> /* Dwarf FL wrappers */
> static char *debuginfo_path; /* Currently dummy */
>
> @@ -177,7 +180,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 +190,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 = 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 +223,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 +233,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 +263,7 @@ static_var:
> }
>
> if (!tvar)
> - return 0;
> + return ret2;
>
> regs = get_arch_regstr(regn);
> if (!regs) {
> @@ -269,7 +282,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 +1281,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 == VARIABLE_LOCATION_INVALID_AT_ADDR) {
> + 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));
> }
>
--
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