Re: [PATCH perf/core v5 11/15] perf probe: Accept %sdt and %cached event name

[Namhyung Kim]

On Thu, Apr 28, 2016 at 03:38:49AM +0900, Masami Hiramatsu wrote:
> From: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> 
> 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 <masami.hiramatsu.pt@xxxxxxxxxxx>
> ---
>  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_<libname> for group name and <provider>_<event> 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_<bin>' 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;
> +
>  	/*
>  	 * <Syntax>
>  	 * 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) &&
>