by Arnaldo Carvalho de Melo

[permalink] [raw]

Subject: Re: [PATCH 16/16] perf probe: Support probing at absolute addresses

Em Wed, Aug 26, 2015 at 12:00:56AM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
> Arnaldo, I don't think we need "+".

What would be the problem? When we write

perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860

What we're meaning is: Take the address where libc-2.19.so is loaded, in
each process, add (+) to that address the number 0xeb860 (an offset),
and when that probe is enabled, put a breakpoint there.

I saw it that way and thus to me it seemed really natural, thus I
applied.

> So, Nak it.

I don't have strong feelings about that one and I saw that there was a
long discussion, so I'll read it and process the patches, if I don't see
anything questionable.

Next time I'll try not to process perf probe patches so fast :-)

- Arnaldo

> > -----Original Message-----
> > From: Arnaldo Carvalho de Melo [mailto:[email protected]]
> > Sent: Wednesday, August 26, 2015 1:15 AM
> > To: Ingo Molnar
> > Cc: [email protected]; Wang Nan; 平松雅巳 / HIRAMATU，MASAMI; Namhyung Kim; [email protected]; Arnaldo Carvalho
> > de Melo
> > Subject: [!][PATCH 16/16] perf probe: Support probing at absolute addresses
> >
> > From: Wang Nan <[email protected]>
> >
> > It should be useful to allow 'perf probe' probe at absolute offsets of a
> > target.
> >
> > For example, when (u)probing at a instruction of a shared object in an
> > embedded system where debuginfo is not available but we know the offset
> > of that instruction by manually digging.
> >
> > This patch enables following perf probe command syntax:
> >
> > # perf probe +0xffffffff811e6615
> >
> > And
> >
> > # perf probe /lib/x86_64-linux-gnu/libc-2.19.so +0xeb860
> >
> > In the above example, we don't need a anchor symbol, so it is possible
> > to compute absolute addresses using other methods and then use 'perf
> > probe' to create the probing points.
> >
> > Committer note:
> >
> > One can as well specify a name, so that the probe doesn't get assigned a
> > generic one starting with abs_, i.e. if we use as described above we
> > get:
> >
> > [root@zoo ~]# readelf -sW /lib64/libc-2.20.so | grep -w malloc
> > 1175: 00000000000830f0 300 FUNC GLOBAL DEFAULT 12 malloc@@GLIBC_2.2.5
> > 813: 0000000000000000 0 FILE LOCAL DEFAULT ABS malloc.c
> > 5276: 00000000000830f0 300 FUNC GLOBAL DEFAULT 12 malloc
> > [root@zoo ~]#
> > [root@zoo ~]# perf probe /lib64/libc-2.20.so +0x830f0
> > Added new event:
> > probe_libc:abs_830f0 (on 0x830f0 in /lib64/libc-2.20.so)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e probe_libc:abs_830f0 -aR sleep 1
> >
> > [root@zoo ~]#
> > [root@zoo ~]# perf probe -l
> > probe_libc:abs_830f0 (on [email protected]/malloc/malloc.c in /lib64/libc-2.20.so)
> > [root@zoo ~]#
> >
> > Whereas if we assign it a name it gets more manageable:
> >
> > [root@zoo ~]# perf probe /lib64/libc-2.20.so malloc=+0x830f0
> > Added new event:
> > probe_libc:malloc (on 0x830f0 in /lib64/libc-2.20.so)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e probe_libc:malloc -aR sleep 1
> >
> > [root@zoo ~]# perf probe -l
> > probe_libc:malloc (on [email protected]/malloc/malloc.c in /lib64/libc-2.20.so)
> > [root@zoo ~]#
> >
> > Both, of course, will produce the same results, and we can use a
> > non canonical name, if the need arises:
> >
> > [root@zoo ~]# perf probe /lib64/libc-2.20.so memory_allocation=+0x830f0
> > Added new event:
> > probe_libc:memory_allocation (on 0x830f0 in /lib64/libc-2.20.so)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e probe_libc:memory_allocation -aR sleep 1
> >
> > [root@zoo ~]# perf probe -l
> > probe_libc:memory_allocation (on [email protected]/malloc/malloc.c in /lib64/libc-2.20.so)
> > [root@zoo ~]#
> >
> > Signed-off-by: Wang Nan <[email protected]>
> > Tested-by: Arnaldo Carvalho de Melo <[email protected]>
> > Cc: Masami Hiramatsu <[email protected]>
> > Cc: Namhyung Kim <[email protected]>
> > Cc: [email protected]
> > Link: http://lkml.kernel.org/r/[email protected]
> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> > ---
> > tools/perf/util/probe-event.c | 144 +++++++++++++++++++++++++++++++++++++----
> > tools/perf/util/probe-event.h | 3 +
> > tools/perf/util/probe-finder.c | 21 +-----
> > 3 files changed, 138 insertions(+), 30 deletions(-)
> >
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 6c7e538c9b8b..59de69a4e3ac 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1194,9 +1194,13 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > *ptr++ = '\0';
> > }
> >
> > - tmp = strdup(arg);
> > - if (tmp == NULL)
> > - return -ENOMEM;
> > + if (arg[0] == '\0')
> > + tmp = NULL;
> > + else {
> > + tmp = strdup(arg);
> > + if (tmp == NULL)
> > + return -ENOMEM;
> > + }
> >
> > if (file_spec)
> > pp->file = tmp;
> > @@ -1283,11 +1287,6 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > return -EINVAL;
> > }
> >
> > - if (pp->offset && !pp->function) {
> > - semantic_error("Offset requires an entry function.\n");
> > - return -EINVAL;
> > - }
> > -
> > if (pp->retprobe && !pp->function) {
> > semantic_error("Return probe requires an entry function.\n");
> > return -EINVAL;
> > @@ -1299,6 +1298,11 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > return -EINVAL;
> > }
> >
> > + if (!pp->function && !pp->offset && !pp->file) {
> > + semantic_error("Absolute address should not be zero.\n");
> > + return -EINVAL;
> > + }
> > +
> > pr_debug("symbol:%s file:%s line:%d offset:%lu return:%d lazy:%s\n",
> > pp->function, pp->file, pp->line, pp->offset, pp->retprobe,
> > pp->lazy_line);
> > @@ -1609,7 +1613,7 @@ error:
> > static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > {
> > char *buf, *tmp;
> > - char offs[32] = "", line[32] = "", file[32] = "";
> > + char offs[32] = "", line[32] = "", file[32] = "", addr[32] = "";
> > int ret, len;
> >
> > buf = zalloc(MAX_CMDLEN);
> > @@ -1622,6 +1626,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > if (ret <= 0)
> > goto error;
> > }
> > + if (!pp->function) {
> > + ret = e_snprintf(addr, 32, "0x%lx", pp->offset);
> > + if (ret <= 0)
> > + goto error;
> > + }
> > if (pp->line) {
> > ret = e_snprintf(line, 32, ":%d", pp->line);
> > if (ret <= 0)
> > @@ -1639,9 +1648,11 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > goto error;
> > }
> >
> > - if (pp->function)
> > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> > - offs, pp->retprobe ? "%return" : "", line,
> > + if (pp->function || pp->offset)
> > + ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s",
> > + pp->function ? : addr,
> > + pp->function ? offs : "",
> > + pp->retprobe ? "%return" : "", line,
> > file);
> > else
> > ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> > @@ -1786,6 +1797,11 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> > if (tev->uprobes)
> > ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> > tp->module, tp->address);
> > + else if (tp->symbol[0] == '0' && tp->symbol[1] == 'x')
> > + /* Absolute address. See try_to_find_absolute_address() */
> > + ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> > + tp->module ?: "", tp->module ? ":" : "",
> > + tp->address);
> > else
> > ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> > tp->module ?: "", tp->module ? ":" : "",
> > @@ -2572,6 +2588,87 @@ err_out:
> > goto out;
> > }
> >
> > +static int try_to_find_absolute_address(struct perf_probe_event *pev,
> > + struct probe_trace_event **tevs)
> > +{
> > + struct perf_probe_point *pp = &pev->point;
> > + struct probe_trace_event *tev;
> > + struct probe_trace_point *tp;
> > + int i, err;
> > +
> > + if (perf_probe_event_need_dwarf(pev) || pev->point.function)
> > + return -EINVAL;
> > +
> > + /*
> > + * This is 'perf probe /lib/libc.so +0xabcd'. Try to probe at
> > + * absolute address.
> > + *
> > + * Only one tev can be generated by this.
> > + */
> > + *tevs = zalloc(sizeof(*tev));
> > + if (!*tevs)
> > + return -ENOMEM;
> > +
> > + tev = *tevs;
> > + tp = &tev->point;
> > +
> > + /*
> > + * Don't use tp->offset, use address directly, because
> > + * in synthesize_probe_trace_command() address cannot be
> > + * zero.
> > + */
> > + tp->address = pev->point.offset;
> > + tp->retprobe = pp->retprobe;
> > + tev->uprobes = pev->uprobes;
> > +
> > + err = -ENOMEM;
> > + /* Give it a '0x' leading symbol name */
> > + if (asprintf(&tp->symbol, "0x%lx", tp->address) < 0)
> > + goto errout;
> > +
> > + /* For kprobe, check range */
> > + if ((!tev->uprobes) &&
> > + (kprobe_warn_out_range(tev->point.symbol,
> > + tev->point.address))) {
> > + err = -EACCES;
> > + goto errout;
> > + }
> > +
> > + if (asprintf(&tp->realname, "abs_%lx", tp->address) < 0)
> > + goto errout;
> > +
> > + if (pev->target) {
> > + tp->module = strdup(pev->target);
> > + if (!tp->module)
> > + goto errout;
> > + }
> > +
> > + if (tev->group) {
> > + tev->group = strdup(pev->group);
> > + if (!tev->group)
> > + goto errout;
> > + }
> > +
> > + if (pev->event) {
> > + tev->event = strdup(pev->event);
> > + if (!tev->event)
> > + goto errout;
> > + }
> > +
> > + tev->nargs = pev->nargs;
> > + for (i = 0; i < tev->nargs; i++)
> > + copy_to_probe_trace_arg(&tev->args[i], &pev->args[i]);
> > +
> > + return 1;
> > +
> > +errout:
> > + if (*tevs) {
> > + clear_probe_trace_events(*tevs, 1);
> > + *tevs = NULL;
> > + }
> > + return err;
> > +}
> > +
> > bool __weak arch__prefers_symtab(void) { return false; }
> >
> > static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> > @@ -2588,6 +2685,10 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> > }
> > }
> >
> > + ret = try_to_find_absolute_address(pev, tevs);
> > + if (ret > 0)
> > + return ret;
> > +
> > if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) {
> > ret = find_probe_trace_events_from_map(pev, tevs);
> > if (ret > 0)
> > @@ -2758,3 +2859,22 @@ end:
> > return ret;
> > }
> >
> > +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> > + struct perf_probe_arg *pvar)
> > +{
> > + tvar->value = strdup(pvar->var);
> > + if (tvar->value == NULL)
> > + return -ENOMEM;
> > + if (pvar->type) {
> > + tvar->type = strdup(pvar->type);
> > + if (tvar->type == NULL)
> > + return -ENOMEM;
> > + }
> > + if (pvar->name) {
> > + tvar->name = strdup(pvar->name);
> > + if (tvar->name == NULL)
> > + return -ENOMEM;
> > + } else
> > + tvar->name = NULL;
> > + return 0;
> > +}
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index 83ee95e9743b..174a3cf6c03d 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -156,4 +156,7 @@ int e_snprintf(char *str, size_t size, const char *format, ...)
> > /* Maximum index number of event-name postfix */
> > #define MAX_EVENT_INDEX 1024
> >
> > +int copy_to_probe_trace_arg(struct probe_trace_arg *tvar,
> > + struct perf_probe_arg *pvar);
> > +
> > #endif /*_PROBE_EVENT_H */
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 7b80f8cb62b9..29c43c0680a8 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -553,24 +553,9 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> > char buf[32], *ptr;
> > int ret = 0;
> >
> > - if (!is_c_varname(pf->pvar->var)) {
> > - /* Copy raw parameters */
> > - pf->tvar->value = strdup(pf->pvar->var);
> > - if (pf->tvar->value == NULL)
> > - return -ENOMEM;
> > - if (pf->pvar->type) {
> > - pf->tvar->type = strdup(pf->pvar->type);
> > - if (pf->tvar->type == NULL)
> > - return -ENOMEM;
> > - }
> > - if (pf->pvar->name) {
> > - pf->tvar->name = strdup(pf->pvar->name);
> > - if (pf->tvar->name == NULL)
> > - return -ENOMEM;
> > - } else
> > - pf->tvar->name = NULL;
> > - return 0;
> > - }
> > + /* Copy raw parameters */
> > + if (!is_c_varname(pf->pvar->var))
> > + return copy_to_probe_trace_arg(pf->tvar, pf->pvar);
> >
> > if (pf->pvar->name)
> > pf->tvar->name = strdup(pf->pvar->name);
> > --
> > 2.1.0
>

2015-08-26 13:39:09

by Arnaldo Carvalho de Melo

[permalink] [raw]

Subject: Re: [GIT PULL 00/16] perf/core improvements and fixes

Em Tue, Aug 25, 2015 at 01:14:22PM -0300, Arnaldo Carvalho de Melo escreveu:
> Hi Ingo,
>
> Please consider pulling, now to get back looking at eBPF :-)

Ingo, please do not process this one, there was a NAK from Masami to
some new syntax on adding non symbolic probes, will resubmit with the
new patches provided by Wang Nan that already got Masami's approval.

- Arnaldo

> - Arnaldo
>
> The following changes since commit 0e53909a1cf0153736fb52c216558a65530d8c40:
>
> Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-08-22 08:45:46 +0200)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
>
> for you to fetch changes up to 1cbebfc7abb549ed3f4080e12c4d7b07d1f16637:
>
> perf probe: Support probing at absolute addresses (2015-08-25 12:36:22 -0300)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> User visible:
>
> - Add support for using several Intel PT features (CYC, MTC packets), the
> relevant documentation was updated: tools/perf/Documentation/intel-pt.txt,
> briefly describing those packets, its purposes, how to configure them in
> the event config terms and relevant external documentation for further
> reading. (Adrian Hunter)
>
> - Introduce support for probing at an absolut address, for user and kernel
> 'perf probe's, useful when one have the symbol maps on a developer machine
> but not on an embedded system (Wang Nan)
>
> - Clear the progress bar at the end of a ordered_events flush, fixing
> an UI artifact when, after ordering the events the screen doesn't get
> completely redraw, for instance, when an error window covers just the
> center of the screen and waits for user input. (Arnaldo Carvalho de Melo)
>
> - Fix 'annotate' segfault by resetting the dso find_symbol cache when removing
> symbols (Arnaldo Carvalho de Melo)
>
> Infrastructure:
>
> - Fix Intel PT 'instructions' sample period (Adrian Hunter)
>
> - Prevent segfault when reading probe point with absolute address (Wang Nan)
>
> Build fixes:
>
> - Fix tarball build broken by pt/bts (Adrian Hunter)
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Adrian Hunter (11):
> perf tools: Fix tarball build broken by pt/bts
> perf tools: Fix Intel PT 'instructions' sample period
> perf tools: Add Intel PT support for PSB periods
> perf tools: Add new Intel PT packet definitions
> perf tools: Pass Intel PT information for decoding MTC and CYC
> perf tools: Add Intel PT support for decoding MTC packets
> perf tools: Add Intel PT support for using MTC packets
> perf tools: Add Intel PT support for decoding CYC packets
> perf tools: Add Intel PT support for using CYC packets
> perf tools: Add Intel PT support for decoding TRACESTOP packets
> perf tools: Update Intel PT documentation
>
> Arnaldo Carvalho de Melo (3):
> perf annotate: Reset the dso find_symbol cache when removing symbols
> perf ui tui progress: Implement the ui_progress_ops->finish() method
> perf ordered_events: Clear the progress bar at the end of a flush
>
> Wang Nan (2):
> perf probe: Prevent segfault when reading probe point with absolute address
> perf probe: Support probing at absolute addresses
>
> tools/perf/Documentation/intel-pt.txt | 194 ++++++-
> tools/perf/arch/x86/util/intel-pt.c | 271 +++++++++-
> tools/perf/builtin-annotate.c | 1 +
> tools/perf/ui/tui/progress.c | 19 +-
> tools/perf/util/dso.h | 2 +
> tools/perf/util/intel-pt-decoder/inat.c | 2 +-
> tools/perf/util/intel-pt-decoder/inat.h | 2 +-
> tools/perf/util/intel-pt-decoder/inat_types.h | 29 ++
> tools/perf/util/intel-pt-decoder/insn.c | 4 +-
> tools/perf/util/intel-pt-decoder/insn.h | 2 +-
> .../perf/util/intel-pt-decoder/intel-pt-decoder.c | 555 ++++++++++++++++++++-
> .../perf/util/intel-pt-decoder/intel-pt-decoder.h | 5 +
> .../util/intel-pt-decoder/intel-pt-insn-decoder.c | 2 +-
> .../util/intel-pt-decoder/intel-pt-pkt-decoder.c | 142 +++++-
> .../util/intel-pt-decoder/intel-pt-pkt-decoder.h | 6 +
> tools/perf/util/intel-pt.c | 67 ++-
> tools/perf/util/intel-pt.h | 5 +
> tools/perf/util/ordered-events.c | 3 +
> tools/perf/util/probe-event.c | 152 +++++-
> tools/perf/util/probe-event.h | 3 +
> tools/perf/util/probe-finder.c | 21 +-
> tools/perf/util/symbol.c | 10 +
> 22 files changed, 1404 insertions(+), 93 deletions(-)
> create mode 100644 tools/perf/util/intel-pt-decoder/inat_types.h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/