[PATCH 7/8] perf probe: Improve detection of file/function name in the probe pattern

[Stefan Bader]

On 11.09.2015 16:42, tim.gardner at canonical.com wrote:
> From: "Naveen N. Rao" <naveen.n.rao at linux.vnet.ibm.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1485528
> 
> Currently, perf probe considers patterns including a '.' to be a file.
> However, this causes problems on powerpc ABIv1 where all functions have
> a leading '.':
> 
>   $ perf probe -F | grep schedule_timeout_interruptible
>   .schedule_timeout_interruptible
>   $ perf probe .schedule_timeout_interruptible
>   Semantic error :File always requires line number or lazy pattern.
>     Error: Command Parse Error.
> 
> Fix this:
> - by checking the probe pattern in more detail, and
> - skipping leading dot if one exists when creating/deleting events.

From the whole set I find that one hard to estimate without knowing more about
general specs. Will the format on all arches require a ";" or ":" and none of
"+@%" to differentiate a file from a function specification? If that is the case
then the change looks ok.

The remainder of patches always seem to insert new (weak) functions with the non
ppc64el path being the same as before. So those seem ok.

-Stefan

> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao at linux.vnet.ibm.com>
> Reviewed-by: Srikar Dronamraju <srikar at linux.vnet.ibm.com>
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt at hitachi.com>
> Cc: Ananth N Mavinakayanahalli <ananth at in.ibm.com>
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: Srikar Dronamraju <srikar at linux.vnet.ibm.com>
> Cc: Sukadev Bhattiprolu <sukadev at linux.vnet.ibm.com>
> Cc: linuxppc-dev at lists.ozlabs.org
> Link: http://lkml.kernel.org/r/db680f7cb11c4452b632f908e67151f3aa0f4602.1430217967.git.naveen.n.rao@linux.vnet.ibm.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme at redhat.com>
> (cherry picked from commit 3099c026002e97b8c173d9d0bbdfc39257d14402)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>  tools/perf/util/probe-event.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 5bd5029..69b6f21 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -973,6 +973,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	struct perf_probe_point *pp = &pev->point;
>  	char *ptr, *tmp;
>  	char c, nc = 0;
> +	bool file_spec = false;
>  	/*
>  	 * <Syntax>
>  	 * perf probe [EVENT=]SRC[:LN|;PTN]
> @@ -1001,6 +1002,23 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		arg = tmp;
>  	}
>  
> +	/*
> +	 * Check arg is function or file name and copy it.
> +	 *
> +	 * We consider arg to be a file spec if and only if it satisfies
> +	 * all of the below criteria::
> +	 * - it does not include any of "+@%",
> +	 * - it includes one of ":;", and
> +	 * - it has a period '.' in the name.
> +	 *
> +	 * Otherwise, we consider arg to be a function specification.
> +	 */
> +	if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
> +		/* This is a file spec if it includes a '.' before ; or : */
> +		if (memchr(arg, '.', ptr - arg))
> +			file_spec = true;
> +	}
> +
>  	ptr = strpbrk(arg, ";:+@%");
>  	if (ptr) {
>  		nc = *ptr;
> @@ -1011,10 +1029,9 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	if (tmp == NULL)
>  		return -ENOMEM;
>  
> -	/* Check arg is function or file and copy it */
> -	if (strchr(tmp, '.'))	/* File */
> +	if (file_spec)
>  		pp->file = tmp;
> -	else			/* Function */
> +	else
>  		pp->function = tmp;
>  
>  	/* Parse other options */
> @@ -2067,6 +2084,9 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
>  {
>  	int i, ret;
>  
> +	if (*base == '.')
> +		base++;
> +
>  	/* Try no suffix */
>  	ret = e_snprintf(buf, len, "%s", base);
>  	if (ret < 0) {
> @@ -2536,6 +2556,9 @@ int del_perf_probe_events(struct strlist *dellist)
>  			event = str;
>  		}
>  
> +		if (event && *event == '.')
> +			event++;
> +
>  		ret = e_snprintf(buf, 128, "%s:%s", group, event);
>  		if (ret < 0) {
>  			pr_err("Failed to copy event.");
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20150915/58aa2807/attachment.sig>