Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752309AbcD1F0X (ORCPT ); Thu, 28 Apr 2016 01:26:23 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:58310 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752170AbcD1F0N (ORCPT ); Thu, 28 Apr 2016 01:26:13 -0400 X-Original-SENDERIP: 156.147.1.121 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 165.244.98.76 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 10.177.227.17 X-Original-MAILFROM: namhyung@kernel.org Date: Thu, 28 Apr 2016 14:26:05 +0900 From: Namhyung Kim To: Masami Hiramatsu CC: Arnaldo Carvalho de Melo , , Peter Zijlstra , Ingo Molnar , Hemant Kumar , Ananth N Mavinakayanahalli Subject: Re: [PATCH perf/core v5 11/15] perf probe: Accept %sdt and %cached event name Message-ID: <20160428052605.GA16129@sejong.aot.lge.com> References: <20160427183701.23446.15293.stgit@devbox> <20160427183849.23446.64132.stgit@devbox> MIME-Version: 1.0 In-Reply-To: <20160427183849.23446.64132.stgit@devbox> User-Agent: Mutt/1.6.0 (2016-04-01) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB05/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/04/28 14:26:05, Serialize by Router on LGEKRMHUB05/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/04/28 14:26:05, Serialize complete at 2016/04/28 14:26:05 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8133 Lines: 242 On Thu, Apr 28, 2016 at 03:38:49AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu > > To improbe usability, support %[PROVIDER:]SDTEVENT format to > add new probes on SDT and cached events. > > e.g. > ---- > # perf probe -x /lib/libc-2.17.so %lll_lock_wait_private > Added new event: > sdt_libc:lll_lock_wait_private (on %lll_lock_wait_private in > /usr/lib/libc-2.17.so) > > You can now use it in all perf tools, such as: > > perf record -e sdt_libc:lll_lock_wait_private -aR sleep 1 > > # perf probe -l | more > sdt_libc:lll_lock_wait_private (on __lll_lock_wait_private+21 > in /usr/lib/libc-2.17.so) > ---- > > Note that this is not only for SDT events, but also normal > events with event-name. > > e.g. define "myevent" on cache (-n doesn't add the real probe) > ---- > # perf probe -x ./perf --cache -n --add 'myevent=dso__load $params' > ---- > Reuse the "myevent" from cache as below. > ---- > # perf probe -x ./perf %myevent > ---- > > TODO: > Wildcard is not supported yet. > > Signed-off-by: Masami Hiramatsu > --- > tools/perf/Documentation/perf-probe.txt | 3 + > tools/perf/util/probe-event.c | 78 ++++++++++++++++++++++--------- > tools/perf/util/probe-event.h | 1 > tools/perf/util/probe-file.c | 9 ++++ > 4 files changed, 69 insertions(+), 22 deletions(-) > > diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt > index 7a258e9..43523be 100644 > --- a/tools/perf/Documentation/perf-probe.txt > +++ b/tools/perf/Documentation/perf-probe.txt > @@ -151,6 +151,8 @@ Probe points are defined by following syntax. > 3) Define event based on source file with lazy pattern > [[GROUP:]EVENT=]SRC;PTN [ARG ...] > > + 4) Pre-defined SDT events > + %[PROVIDER:]SDTEVENT How about changing it to: 4) Pre-defined (SDT) or cached events %EVENTNAME ? Btw, is it possible to use group name here? The SDT will use sdt_ for group name and _ for event name, right? So is it for cached events and does it support defining group name when added like below? # perf probe -x ./perf --cache --add 'mygroup:myevent=main' Thanks, Namhyung > > 'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function. You can also specify a group name by 'GROUP', if omitted, set 'probe' is used for kprobe and 'probe_' is used for uprobe. > Note that using existing group name can conflict with other events. Especially, using the group name reserved for kernel modules can hide embedded events in the > @@ -158,6 +160,7 @@ modules. > 'FUNC' specifies a probed function name, and it may have one of the following options; '+OFFS' is the offset from function entry address in bytes, ':RLN' is the relative-line number from function entry line, and '%return' means that it probes function return. And ';PTN' means lazy matching pattern (see LAZY MATCHING). Note that ';PTN' must be the end of the probe point definition. In addition, '@SRC' specifies a source file which has that function. > It is also possible to specify a probe point by the source line number or lazy matching by using 'SRC:ALN' or 'SRC;PTN' syntax, where 'SRC' is the source file path, ':ALN' is the line number and ';PTN' is the lazy matching pattern. > 'ARG' specifies the arguments of this probe point, (see PROBE ARGUMENT). > +'SDTEVENT' and 'PROVIDER' is the pre-defined event name which is defined by user SDT (Statically Defined Tracing) or the pre-cached probes with event name. > > PROBE ARGUMENT > -------------- > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index b2c6a4a..1a9ea2b 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1199,6 +1199,34 @@ err: > return err; > } > > +static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev) > +{ > + char *ptr; > + > + ptr = strchr(*arg, ':'); > + if (ptr) { > + *ptr = '\0'; > + if (!is_c_func_name(*arg)) > + goto ng_name; > + pev->group = strdup(*arg); > + if (!pev->group) > + return -ENOMEM; > + *arg = ptr + 1; > + } else > + pev->group = NULL; > + if (!is_c_func_name(*arg)) { > +ng_name: > + semantic_error("%s is bad for event name -it must " > + "follow C symbol-naming rule.\n", *arg); > + return -EINVAL; > + } > + pev->event = strdup(*arg); > + if (pev->event == NULL) > + return -ENOMEM; > + > + return 0; > +} > + > /* Parse probepoint definition. */ > static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > { > @@ -1206,38 +1234,43 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > char *ptr, *tmp; > char c, nc = 0; > bool file_spec = false; > + int ret; > + > /* > * > * perf probe [GRP:][EVENT=]SRC[:LN|;PTN] > * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT] > + * perf probe %[GRP:]SDT_EVENT > */ > if (!arg) > return -EINVAL; > > + if (arg[0] == '%') { > + pev->sdt = true; > + arg++; > + } > + > ptr = strpbrk(arg, ";=@+%"); > - if (ptr && *ptr == '=') { /* Event name */ > - *ptr = '\0'; > - tmp = ptr + 1; > - ptr = strchr(arg, ':'); > + if (pev->sdt) { > if (ptr) { > - *ptr = '\0'; > - if (!is_c_func_name(arg)) > - goto not_fname; > - pev->group = strdup(arg); > - if (!pev->group) > - return -ENOMEM; > - arg = ptr + 1; > - } else > - pev->group = NULL; > - if (!is_c_func_name(arg)) { > -not_fname: > - semantic_error("%s is bad for event name -it must " > - "follow C symbol-naming rule.\n", arg); > + semantic_error("%s must contain only an SDT event name.\n", arg); > return -EINVAL; > } > - pev->event = strdup(arg); > - if (pev->event == NULL) > - return -ENOMEM; > + ret = parse_perf_probe_event_name(&arg, pev); > + if (ret == 0) { > + if (asprintf(&pev->point.function, "%%%s", pev->event) < 0) > + ret = -errno; > + } > + return ret; > + } > + > + if (ptr && *ptr == '=') { /* Event name */ > + *ptr = '\0'; > + tmp = ptr + 1; > + ret = parse_perf_probe_event_name(&arg, pev); > + if (ret < 0) > + return ret; > + > arg = tmp; > } > > @@ -2849,7 +2882,8 @@ static int find_probe_trace_events_from_cache(struct perf_probe_event *pev, > pev->point.function); > } > if (!entry) { > - ret = 0; > + /* SDT must be in the cache */ > + ret = pev->sdt ? -ENOENT : 0; > goto out; > } > > @@ -2888,7 +2922,7 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev, > { > int ret; > > - if (!pev->group) { > + if (!pev->group && !pev->sdt) { > /* Set group name if not given */ > if (!pev->uprobes) { > pev->group = strdup(PERFPROBE_GROUP); > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > index c451223..2a23efe 100644 > --- a/tools/perf/util/probe-event.h > +++ b/tools/perf/util/probe-event.h > @@ -85,6 +85,7 @@ struct perf_probe_event { > char *group; /* Group name */ > struct perf_probe_point point; /* Probe point */ > int nargs; /* Number of arguments */ > + bool sdt; /* SDT/cached event flag */ > bool uprobes; /* Uprobe event flag */ > char *target; /* Target binary */ > struct perf_probe_arg *args; /* Arguments */ > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > index c10a647..ee560dd 100644 > --- a/tools/perf/util/probe-file.c > +++ b/tools/perf/util/probe-file.c > @@ -529,6 +529,15 @@ probe_cache__find(struct probe_cache *pcache, struct perf_probe_event *pev) > return NULL; > > list_for_each_entry(entry, &pcache->list, list) { > + if (pev->sdt) { > + if (entry->pev.event && > + streql(entry->pev.event, pev->event) && > + (!pev->group || > + streql(entry->pev.group, pev->group))) > + goto found; > + > + continue; > + } > /* Hit if same event name or same command-string */ > if ((pev->event && > (streql(entry->pev.group, pev->group) && >