Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752088AbbEKCXY (ORCPT ); Sun, 10 May 2015 22:23:24 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:37902 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751435AbbEKCXX (ORCPT ); Sun, 10 May 2015 22:23:23 -0400 Subject: Re: [PATCH v2 2/3] perf probe: Add --range option to show variable location range To: Masami Hiramatsu , , , , References: <1431165306-106463-1-git-send-email-hekuang@huawei.com> <1431165306-106463-2-git-send-email-hekuang@huawei.com> <554ECEB9.2040209@hitachi.com> CC: , From: He Kuang Message-ID: <5550127D.7020506@huawei.com> Date: Mon, 11 May 2015 10:22:53 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.0 MIME-Version: 1.0 In-Reply-To: <554ECEB9.2040209@hitachi.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.110.54.65] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.55501292.0077,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 8c44b6f64d57b28574a36c9e8c27ad92 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7407 Lines: 206 Hi, Masami On 2015/5/10 11:21, Masami Hiramatsu wrote: > 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(): >> >> >> 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 >> @ >> [INV] ssize_t written @ >> [INV] struct address_space_operations* a_ops @ >> [VAL] (unknown_type) fsdata @ >> [VAL] loff_t pos @ >> [VAL] long int status @ >> [VAL] long unsigned int bytes @ >> [VAL] struct address_space* mapping @ >> [VAL] struct iov_iter* i @ >> [VAL] struct page* page @ >> > 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! I've checked libdw, it never returns -ERANGE, but there is an errno conflict in the function convert_variable_location itself: 268 regs = get_arch_regstr(regn); 269 if (!regs) { 270 /* This should be a bug in DWARF or this tool */ 271 pr_warning("Mapping for the register number %u " 272 "missing on this architecture.\n", regn); 273 return -ERANGE; 274 } So shell we change the above errno to -ENOENT or choose another errno for current 'VARIABLE_LOCATION_INVALID_AT_ADDR', what's your opinion? Thanks! > > >> + >> /* 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)); >> } >> > -- 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/