[PATCH -tip 0/4] perf/probe: Improve error messages

[PATCH -tip 0/4] perf/probe: Improve error messages

[Masami Hiramatsu]

Hi,

Here is a series that improves error messages of perf probe.

 -  Improve the error message if we can not find given member in
    the given structure. (perf probe --add)
 -  Show error code and description only in verbose mode if
    perf probe command is failed.
 -  Improve error messages of perf probe --vars mode.
 -  Improve error messages of perf probe --line mode.

This series is mainly for fixing confusing messages and
for removing the error code which is just a mysterious
number for users.

Thank you,

---

Masami Hiramatsu (4):
      perf/probe: Improve error message for unknown member of data structure
      perf/probe: Show error code and description in verbose mode
      perf/probe: Improve an error message of perf probe --vars mode
      perf/probe: Improve error messages with --line option


 tools/perf/builtin-probe.c     |   23 ++++++++++++++---------
 tools/perf/util/probe-event.c  |   13 +++++++++----
 tools/perf/util/probe-finder.c |   11 +++++++----
 3 files changed, 30 insertions(+), 17 deletions(-)

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH -tip 1/4] perf/probe: Improve error message for unknown member of data structure

[Masami Hiramatsu]

Improve the error message if we can not find given member in
the given structure. Currently perf probe shows a wrong error
message as below.

  -----
  # perf probe getname_flags:65 "result->BOGUS"
  result(type:filename) has no member BOGUS.
  Failed to find 'result' in this function.
    Error: Failed to add events. (-22)
  -----

The first message is correct, but the second one is not, since
we didn't fail to find a variable but fails to find the member
of given variable.

  -----
  # perf probe getname_flags:65 "result->BOGUS"
  result(type:filename) has no member BOGUS.
    Error: Failed to add events. (-22)
  -----

With this patch, the error message shows only the first one.
And if we really failed to find given variable, it tells us so.

  -----
  # perf probe getname_flags:65 "BOGUS"
  Failed to find 'BOGUS' in this function.
    Error: Failed to add events. (-2)
  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/probe-finder.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9d8eb26..ce8faf4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
 	if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
 		/* Search again in global variables */
 		if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
+			pr_warning("Failed to find '%s' in this function.\n",
+				   pf->pvar->var);
 			ret = -ENOENT;
 	}
 	if (ret >= 0)
 		ret = convert_variable(&vr_die, pf);
 
-	if (ret < 0)
-		pr_warning("Failed to find '%s' in this function.\n",
-			   pf->pvar->var);
 	return ret;
 }
 

[tip:perf/core] perf probe: Improve error message for unknown member of data structure

[tip-bot for Masami Hiramatsu]

Commit-ID:  36d789a4d75f3826faa6e75b018942b63ffed1a0
Gitweb:     http://git.kernel.org/tip/36d789a4d75f3826faa6e75b018942b63ffed1a0
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Jun 2014 07:13:45 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 9 Jun 2014 12:15:07 -0300

perf probe: Improve error message for unknown member of data structure

Improve the error message if we can not find given member in the given
structure. Currently perf probe shows a wrong error message as below.

  -----
  # perf probe getname_flags:65 "result->BOGUS"
  result(type:filename) has no member BOGUS.
  Failed to find 'result' in this function.
    Error: Failed to add events. (-22)
  -----

The first message is correct, but the second one is not, since we didn't
fail to find a variable but fails to find the member of given variable.

  -----
  # perf probe getname_flags:65 "result->BOGUS"
  result(type:filename) has no member BOGUS.
    Error: Failed to add events. (-22)
  -----

With this patch, the error message shows only the first one.  And if we
really failed to find given variable, it tells us so.

  -----
  # perf probe getname_flags:65 "BOGUS"
  Failed to find 'BOGUS' in this function.
    Error: Failed to add events. (-2)
  -----

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20140606071345.6788.23744.stgit@kbuild-fedora.novalocal
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9d8eb26..ce8faf4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
 	if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
 		/* Search again in global variables */
 		if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
+			pr_warning("Failed to find '%s' in this function.\n",
+				   pf->pvar->var);
 			ret = -ENOENT;
 	}
 	if (ret >= 0)
 		ret = convert_variable(&vr_die, pf);
 
-	if (ret < 0)
-		pr_warning("Failed to find '%s' in this function.\n",
-			   pf->pvar->var);
 	return ret;
 }
 

Re: [PATCH -tip 1/4] perf/probe: Improve error message for unknown member of data structure

[Patrick Palka]

On Fri, Jun 6, 2014 at 3:13 AM, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
> Improve the error message if we can not find given member in
> the given structure. Currently perf probe shows a wrong error
> message as below.
>
>   -----
>   # perf probe getname_flags:65 "result->BOGUS"
>   result(type:filename) has no member BOGUS.
>   Failed to find 'result' in this function.
>     Error: Failed to add events. (-22)
>   -----
>
> The first message is correct, but the second one is not, since
> we didn't fail to find a variable but fails to find the member
> of given variable.
>
>   -----
>   # perf probe getname_flags:65 "result->BOGUS"
>   result(type:filename) has no member BOGUS.
>     Error: Failed to add events. (-22)
>   -----
>
> With this patch, the error message shows only the first one.
> And if we really failed to find given variable, it tells us so.
>
>   -----
>   # perf probe getname_flags:65 "BOGUS"
>   Failed to find 'BOGUS' in this function.
>     Error: Failed to add events. (-2)
>   -----
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> ---
>  tools/perf/util/probe-finder.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 9d8eb26..ce8faf4 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
>         if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
>                 /* Search again in global variables */
>                 if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
> +                       pr_warning("Failed to find '%s' in this function.\n",
> +                                  pf->pvar->var);
>                         ret = -ENOENT;

It looks like you're missing a pair of braces here.

Re: [PATCH -tip 1/4] perf/probe: Improve error message for unknown member of data structure

[Masami Hiramatsu]

(2014/06/22 3:18), Patrick Palka wrote:
> On Fri, Jun 6, 2014 at 3:13 AM, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>> Improve the error message if we can not find given member in
>> the given structure. Currently perf probe shows a wrong error
>> message as below.
>>
>>   -----
>>   # perf probe getname_flags:65 "result->BOGUS"
>>   result(type:filename) has no member BOGUS.
>>   Failed to find 'result' in this function.
>>     Error: Failed to add events. (-22)
>>   -----
>>
>> The first message is correct, but the second one is not, since
>> we didn't fail to find a variable but fails to find the member
>> of given variable.
>>
>>   -----
>>   # perf probe getname_flags:65 "result->BOGUS"
>>   result(type:filename) has no member BOGUS.
>>     Error: Failed to add events. (-22)
>>   -----
>>
>> With this patch, the error message shows only the first one.
>> And if we really failed to find given variable, it tells us so.
>>
>>   -----
>>   # perf probe getname_flags:65 "BOGUS"
>>   Failed to find 'BOGUS' in this function.
>>     Error: Failed to add events. (-2)
>>   -----
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> ---
>>  tools/perf/util/probe-finder.c |    5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index 9d8eb26..ce8faf4 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
>>         if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
>>                 /* Search again in global variables */
>>                 if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
>> +                       pr_warning("Failed to find '%s' in this function.\n",
>> +                                  pf->pvar->var);
>>                         ret = -ENOENT;
> 
> It looks like you're missing a pair of braces here.

Oops, right! Thank you very much!

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH -tip ] [BUGFIX]: Fix to add a missing pair of braces for error path.

[Masami Hiramatsu]

Fix to add a missing pair of braces for error path.
Commit 36d789a4d75f (perf probe: Improve error message for
unknown member of data structure) introduced this bug.

Without this fix, defining an event with global variables
is always failed, because it always returns -ENOENT if
the argument is not a local variable.

  ----
  # perf probe -na "vfs_read smp_found_config"
    Error: Failed to add events.
  ----

With this fix, you can set a global variable for the
argument of new event.

  ----
  # perf probe -na "vfs_read smp_found_config"
  Added new event:
    probe:vfs_read       (on vfs_read with smp_found_config)

  You can now use it in all perf tools, such as:

          perf record -e probe:vfs_read -aR sleep 1
  ----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reported-by: Patrick Palka <patrick@parcs.ath.cx>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-finder.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 98e3047..10560fc 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -572,10 +572,12 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
 	/* Search child die for local variables and parameters. */
 	if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
 		/* Search again in global variables */
-		if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
+		if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0,
+					  &vr_die)) {
 			pr_warning("Failed to find '%s' in this function.\n",
 				   pf->pvar->var);
 			ret = -ENOENT;
+		}
 	}
 	if (ret >= 0)
 		ret = convert_variable(&vr_die, pf);

Re: [PATCH -tip ] [BUGFIX]: Fix to add a missing pair of braces for error path.

[Namhyung Kim]

Hi Masami,

On Mon, 23 Jun 2014 03:17:12 +0000, Masami Hiramatsu wrote:
> Fix to add a missing pair of braces for error path.
> Commit 36d789a4d75f (perf probe: Improve error message for
> unknown member of data structure) introduced this bug.
>
> Without this fix, defining an event with global variables
> is always failed, because it always returns -ENOENT if
> the argument is not a local variable.
>
>   ----
>   # perf probe -na "vfs_read smp_found_config"
>     Error: Failed to add events.
>   ----
>
> With this fix, you can set a global variable for the
> argument of new event.
>
>   ----
>   # perf probe -na "vfs_read smp_found_config"
>   Added new event:
>     probe:vfs_read       (on vfs_read with smp_found_config)
>
>   You can now use it in all perf tools, such as:
>
>           perf record -e probe:vfs_read -aR sleep 1
>   ----
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Reported-by: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

Re: [PATCH -tip ] [BUGFIX]: Fix to add a missing pair of braces for error path.

[Jiri Olsa]

On Mon, Jun 23, 2014 at 03:17:12AM +0000, Masami Hiramatsu wrote:
> Fix to add a missing pair of braces for error path.
> Commit 36d789a4d75f (perf probe: Improve error message for
> unknown member of data structure) introduced this bug.
> 
> Without this fix, defining an event with global variables
> is always failed, because it always returns -ENOENT if
> the argument is not a local variable.
> 
>   ----
>   # perf probe -na "vfs_read smp_found_config"
>     Error: Failed to add events.
>   ----
> 
> With this fix, you can set a global variable for the
> argument of new event.
> 
>   ----
>   # perf probe -na "vfs_read smp_found_config"
>   Added new event:
>     probe:vfs_read       (on vfs_read with smp_found_config)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:vfs_read -aR sleep 1
>   ----

queued for perf/urgent

thanks,
jirka

[PATCH -tip 2/4] perf/probe: Show error code and description in verbose mode

[Masami Hiramatsu]

Show error code and description only in verbose mode if
perf probe command is failed. Current perf probe shows
error code with final error message, and that is meaningless
for many users. This changes error messages to show the error
code and its description only in verbose mode (-v option).

Without this patch:
  -----
  # perf probe -a do_execve@hoge
  Probe point 'do_execve@hoge' not found.
    Error: Failed to add events. (-2)
  -----

With this patch, normally the message doesn't show the
misterious error number.
  -----
  # perf probe -a do_execve@hoge
  Probe point 'do_execve@hoge' not found.
    Error: Failed to add events.
  -----

And in verbose mode, it also shows additional error
messages as below:
  -----
  # perf probe -va do_execve@hoge
  probe-definition(0): do_execve@hoge
  symbol:do_execve file:hoge line:0 offset:0 return:0 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (6 entries long)
  Using /lib/modules/3.15.0-rc8+/build/vmlinux for symbols
  Open Debuginfo file: /lib/modules/3.15.0-rc8+/build/vmlinux
  Try to find probe point from debuginfo.
  Probe point 'do_execve@hoge' not found.
    Error: Failed to add events. Reason: No such file or directory (Code: -2)
  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
---
 tools/perf/builtin-probe.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index cdcd4eb..c63fa29 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -288,6 +288,13 @@ static void cleanup_params(void)
 	memset(&params, 0, sizeof(params));
 }
 
+static void pr_err_with_code(const char *msg, int err)
+{
+	pr_err("%s", msg);
+	pr_debug(" Reason: %s (Code: %d)", strerror(-err), err);
+	pr_err("\n");
+}
+
 static int
 __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 {
@@ -379,7 +386,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		ret = parse_probe_event_argv(argc, argv);
 		if (ret < 0) {
-			pr_err("  Error: Parse Error.  (%d)\n", ret);
+			pr_err_with_code("  Error: Command Parse Error.", ret);
 			return ret;
 		}
 	}
@@ -419,8 +426,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		ret = show_perf_probe_events();
 		if (ret < 0)
-			pr_err("  Error: Failed to show event list. (%d)\n",
-			       ret);
+			pr_err_with_code("  Error: Failed to show event list.", ret);
 		return ret;
 	}
 	if (params.show_funcs) {
@@ -445,8 +451,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		strfilter__delete(params.filter);
 		params.filter = NULL;
 		if (ret < 0)
-			pr_err("  Error: Failed to show functions."
-			       " (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to show functions.", ret);
 		return ret;
 	}
 
@@ -464,7 +469,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 
 		ret = show_line_range(&params.line_range, params.target);
 		if (ret < 0)
-			pr_err("  Error: Failed to show lines. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to show lines.", ret);
 		return ret;
 	}
 	if (params.show_vars) {
@@ -485,7 +490,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		strfilter__delete(params.filter);
 		params.filter = NULL;
 		if (ret < 0)
-			pr_err("  Error: Failed to show vars. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to show vars.", ret);
 		return ret;
 	}
 #endif
@@ -493,7 +498,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (params.dellist) {
 		ret = del_perf_probe_events(params.dellist);
 		if (ret < 0) {
-			pr_err("  Error: Failed to delete events. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to delete events.", ret);
 			return ret;
 		}
 	}
@@ -504,7 +509,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 					    params.target,
 					    params.force_add);
 		if (ret < 0) {
-			pr_err("  Error: Failed to add events. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to add events.", ret);
 			return ret;
 		}
 	}

[tip:perf/core] perf probe: Show error code and description in verbose mode

[tip-bot for Masami Hiramatsu]

Commit-ID:  b4bf1130cdee7d5247bd3171530869809f5aca54
Gitweb:     http://git.kernel.org/tip/b4bf1130cdee7d5247bd3171530869809f5aca54
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Jun 2014 07:13:52 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 9 Jun 2014 14:34:09 -0300

perf probe: Show error code and description in verbose mode

Show error code and description only in verbose mode if 'perf probe'
command failed.

Current 'perf probe' shows error code with final error message, and that
is meaningless for many users.

This changes error messages to show the error code and its description
only in verbose mode (-v option).

Without this patch:
  -----
  # perf probe -a do_execve@hoge
  Probe point 'do_execve@hoge' not found.
    Error: Failed to add events. (-2)
  -----

With this patch, normally the message doesn't show the misterious error
number:
  -----
  # perf probe -a do_execve@hoge
  Probe point 'do_execve@hoge' not found.
    Error: Failed to add events.
  -----

And in verbose mode, it also shows additional error messages as below:
  -----
  # perf probe -va do_execve@hoge
  probe-definition(0): do_execve@hoge
  symbol:do_execve file:hoge line:0 offset:0 return:0 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (6 entries long)
  Using /lib/modules/3.15.0-rc8+/build/vmlinux for symbols
  Open Debuginfo file: /lib/modules/3.15.0-rc8+/build/vmlinux
  Try to find probe point from debuginfo.
  Probe point 'do_execve@hoge' not found.
    Error: Failed to add events. Reason: No such file or directory (Code: -2)
  -----

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20140606071352.6788.76943.stgit@kbuild-fedora.novalocal
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-probe.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index cdcd4eb..c63fa29 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -288,6 +288,13 @@ static void cleanup_params(void)
 	memset(&params, 0, sizeof(params));
 }
 
+static void pr_err_with_code(const char *msg, int err)
+{
+	pr_err("%s", msg);
+	pr_debug(" Reason: %s (Code: %d)", strerror(-err), err);
+	pr_err("\n");
+}
+
 static int
 __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 {
@@ -379,7 +386,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		ret = parse_probe_event_argv(argc, argv);
 		if (ret < 0) {
-			pr_err("  Error: Parse Error.  (%d)\n", ret);
+			pr_err_with_code("  Error: Command Parse Error.", ret);
 			return ret;
 		}
 	}
@@ -419,8 +426,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		ret = show_perf_probe_events();
 		if (ret < 0)
-			pr_err("  Error: Failed to show event list. (%d)\n",
-			       ret);
+			pr_err_with_code("  Error: Failed to show event list.", ret);
 		return ret;
 	}
 	if (params.show_funcs) {
@@ -445,8 +451,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		strfilter__delete(params.filter);
 		params.filter = NULL;
 		if (ret < 0)
-			pr_err("  Error: Failed to show functions."
-			       " (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to show functions.", ret);
 		return ret;
 	}
 
@@ -464,7 +469,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 
 		ret = show_line_range(&params.line_range, params.target);
 		if (ret < 0)
-			pr_err("  Error: Failed to show lines. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to show lines.", ret);
 		return ret;
 	}
 	if (params.show_vars) {
@@ -485,7 +490,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		strfilter__delete(params.filter);
 		params.filter = NULL;
 		if (ret < 0)
-			pr_err("  Error: Failed to show vars. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to show vars.", ret);
 		return ret;
 	}
 #endif
@@ -493,7 +498,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (params.dellist) {
 		ret = del_perf_probe_events(params.dellist);
 		if (ret < 0) {
-			pr_err("  Error: Failed to delete events. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to delete events.", ret);
 			return ret;
 		}
 	}
@@ -504,7 +509,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 					    params.target,
 					    params.force_add);
 		if (ret < 0) {
-			pr_err("  Error: Failed to add events. (%d)\n", ret);
+			pr_err_with_code("  Error: Failed to add events.", ret);
 			return ret;
 		}
 	}

[PATCH -tip 3/4] perf/probe: Improve an error message of perf probe --vars mode

[Masami Hiramatsu]

Fix an error message when failed to find given address in --vars
mode.

Without this fix, perf probe -V doesn't show the final "Error"
message if it fails to find given source line. Moreover, it
tells it fails to find "variables" instead of the source line.
  -----
  # perf probe -V foo@bar
  Failed to find variables at foo@bar (0)
  -----
The result also shows mysterious error code. Actually the error
returns 0 or -ENOENT means that it just fails to find the address
of given source line. (0 means there is no matching address,
and -ENOENT means there is an entry(DIE) but it has no instance,
e.g. an empty inlined function)

This fixes it to show what happened and the final error message
as below.
  -----
  # perf probe -V foo@bar
  Failed to find the address of foo@bar
    Error: Failed to show vars.
  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/probe-event.c  |    7 ++++++-
 tools/perf/util/probe-finder.c |    6 +++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0d1542f..44c7141 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -721,9 +721,14 @@ static int show_available_vars_at(struct debuginfo *dinfo,
 	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
 						max_vls, externs);
 	if (ret <= 0) {
-		pr_err("Failed to find variables at %s (%d)\n", buf, ret);
+		if (ret == 0 || ret == -ENOENT) {
+			pr_err("Failed to find the address of %s\n", buf);
+			ret = -ENOENT;
+		} else
+			pr_warning("Debuginfo analysis failed.\n");
 		goto end;
 	}
+
 	/* Some variables are found */
 	fprintf(stdout, "Available variables at %s\n", buf);
 	for (i = 0; i < ret; i++) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ce8faf4..98e3047 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1280,7 +1280,11 @@ out:
 	return ret;
 }
 
-/* Find available variables at given probe point */
+/*
+ * Find available variables at given probe point
+ * Return the number of found probe points. Return 0 if there is no
+ * matched probe point. Return <0 if an error occurs.
+ */
 int debuginfo__find_available_vars_at(struct debuginfo *dbg,
 				      struct perf_probe_event *pev,
 				      struct variable_list **vls,

[tip:perf/core] perf probe: Improve an error message of perf probe --vars mode

[tip-bot for Masami Hiramatsu]

Commit-ID:  69e96eaa4fef04ad543eda3eab787dbae99d8912
Gitweb:     http://git.kernel.org/tip/69e96eaa4fef04ad543eda3eab787dbae99d8912
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Jun 2014 07:13:59 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 9 Jun 2014 14:35:58 -0300

perf probe: Improve an error message of perf probe --vars mode

Fix an error message when failed to find given address in --vars
mode.

Without this fix, perf probe -V doesn't show the final "Error"
message if it fails to find given source line. Moreover, it
tells it fails to find "variables" instead of the source line.
  -----
  # perf probe -V foo@bar
  Failed to find variables at foo@bar (0)
  -----
The result also shows mysterious error code. Actually the error
returns 0 or -ENOENT means that it just fails to find the address
of given source line. (0 means there is no matching address,
and -ENOENT means there is an entry(DIE) but it has no instance,
e.g. an empty inlined function)

This fixes it to show what happened and the final error message
as below.
  -----
  # perf probe -V foo@bar
  Failed to find the address of foo@bar
    Error: Failed to show vars.
  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20140606071359.6788.84716.stgit@kbuild-fedora.novalocal
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c  | 7 ++++++-
 tools/perf/util/probe-finder.c | 6 +++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0d1542f..44c7141 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -721,9 +721,14 @@ static int show_available_vars_at(struct debuginfo *dinfo,
 	ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
 						max_vls, externs);
 	if (ret <= 0) {
-		pr_err("Failed to find variables at %s (%d)\n", buf, ret);
+		if (ret == 0 || ret == -ENOENT) {
+			pr_err("Failed to find the address of %s\n", buf);
+			ret = -ENOENT;
+		} else
+			pr_warning("Debuginfo analysis failed.\n");
 		goto end;
 	}
+
 	/* Some variables are found */
 	fprintf(stdout, "Available variables at %s\n", buf);
 	for (i = 0; i < ret; i++) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ce8faf4..98e3047 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1280,7 +1280,11 @@ out:
 	return ret;
 }
 
-/* Find available variables at given probe point */
+/*
+ * Find available variables at given probe point
+ * Return the number of found probe points. Return 0 if there is no
+ * matched probe point. Return <0 if an error occurs.
+ */
 int debuginfo__find_available_vars_at(struct debuginfo *dbg,
 				      struct perf_probe_event *pev,
 				      struct variable_list **vls,

[PATCH -tip 4/4] perf/probe: Improve error messages with --line option

[Masami Hiramatsu]

Improve error messages of perf probe --line mode. Current
perf probe shows "Debuginfo analysis failed" error with an
error code when the given symbol is not found as below.

  -----
  # perf probe -L page_cgroup_init_flatmem
  Debuginfo analysis failed. (-2)
    Error: Failed to show lines.
  -----

But -2 (-ENOMEM) means that the given source line or function
is not found. With this patch, perf probe shows correct error
message as below.

  -----
  # perf probe -L page_cgroup_init_flatmem
  Specified source line is not found.
    Error: Failed to show lines.
  -----

There is also another debug error code is shown in the same
function after get_real_path(). This removes that too.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
 tools/perf/util/probe-event.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 44c7141..9a0a183 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
 
 	ret = debuginfo__find_line_range(dinfo, lr);
 	debuginfo__delete(dinfo);
-	if (ret == 0) {
+	if (ret == 0 || ret == -ENOENT) {
 		pr_warning("Specified source line is not found.\n");
 		return -ENOENT;
 	} else if (ret < 0) {
-		pr_warning("Debuginfo analysis failed. (%d)\n", ret);
+		pr_warning("Debuginfo analysis failed.\n");
 		return ret;
 	}
 
@@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
 	ret = get_real_path(tmp, lr->comp_dir, &lr->path);
 	free(tmp);	/* Free old path */
 	if (ret < 0) {
-		pr_warning("Failed to find source file. (%d)\n", ret);
+		pr_warning("Failed to find source file path.\n");
 		return ret;
 	}
 

Re: [PATCH -tip 4/4] perf/probe: Improve error messages with --line option

[Namhyung Kim]

Hi Masami,

On Fri, 06 Jun 2014 07:14:06 +0000, Masami Hiramatsu wrote:
> Improve error messages of perf probe --line mode. Current
> perf probe shows "Debuginfo analysis failed" error with an
> error code when the given symbol is not found as below.
>
>   -----
>   # perf probe -L page_cgroup_init_flatmem
>   Debuginfo analysis failed. (-2)
>     Error: Failed to show lines.
>   -----
>
> But -2 (-ENOMEM) means that the given source line or function

s/ENOMEM/ENOENT/

Thanks,
Namhyung


> is not found. With this patch, perf probe shows correct error
> message as below.
>
>   -----
>   # perf probe -L page_cgroup_init_flatmem
>   Specified source line is not found.
>     Error: Failed to show lines.
>   -----
>
> There is also another debug error code is shown in the same
> function after get_real_path(). This removes that too.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> ---
>  tools/perf/util/probe-event.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 44c7141..9a0a183 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
>  
>  	ret = debuginfo__find_line_range(dinfo, lr);
>  	debuginfo__delete(dinfo);
> -	if (ret == 0) {
> +	if (ret == 0 || ret == -ENOENT) {
>  		pr_warning("Specified source line is not found.\n");
>  		return -ENOENT;
>  	} else if (ret < 0) {
> -		pr_warning("Debuginfo analysis failed. (%d)\n", ret);
> +		pr_warning("Debuginfo analysis failed.\n");
>  		return ret;
>  	}
>  
> @@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
>  	ret = get_real_path(tmp, lr->comp_dir, &lr->path);
>  	free(tmp);	/* Free old path */
>  	if (ret < 0) {
> -		pr_warning("Failed to find source file. (%d)\n", ret);
> +		pr_warning("Failed to find source file path.\n");
>  		return ret;
>  	}
>  

Re: Re: [PATCH -tip 4/4] perf/probe: Improve error messages with --line option

[Masami Hiramatsu]

(2014/06/10 17:11), Namhyung Kim wrote:
> Hi Masami,
> 
> On Fri, 06 Jun 2014 07:14:06 +0000, Masami Hiramatsu wrote:
>> Improve error messages of perf probe --line mode. Current
>> perf probe shows "Debuginfo analysis failed" error with an
>> error code when the given symbol is not found as below.
>>
>>   -----
>>   # perf probe -L page_cgroup_init_flatmem
>>   Debuginfo analysis failed. (-2)
>>     Error: Failed to show lines.
>>   -----
>>
>> But -2 (-ENOMEM) means that the given source line or function
> 
> s/ENOMEM/ENOENT/

Ah, right :) It's a typo...

Thanks!

> 
> Thanks,
> Namhyung
> 
> 
>> is not found. With this patch, perf probe shows correct error
>> message as below.
>>
>>   -----
>>   # perf probe -L page_cgroup_init_flatmem
>>   Specified source line is not found.
>>     Error: Failed to show lines.
>>   -----
>>
>> There is also another debug error code is shown in the same
>> function after get_real_path(). This removes that too.
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> ---
>>  tools/perf/util/probe-event.c |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 44c7141..9a0a183 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
>>  
>>  	ret = debuginfo__find_line_range(dinfo, lr);
>>  	debuginfo__delete(dinfo);
>> -	if (ret == 0) {
>> +	if (ret == 0 || ret == -ENOENT) {
>>  		pr_warning("Specified source line is not found.\n");
>>  		return -ENOENT;
>>  	} else if (ret < 0) {
>> -		pr_warning("Debuginfo analysis failed. (%d)\n", ret);
>> +		pr_warning("Debuginfo analysis failed.\n");
>>  		return ret;
>>  	}
>>  
>> @@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
>>  	ret = get_real_path(tmp, lr->comp_dir, &lr->path);
>>  	free(tmp);	/* Free old path */
>>  	if (ret < 0) {
>> -		pr_warning("Failed to find source file. (%d)\n", ret);
>> +		pr_warning("Failed to find source file path.\n");
>>  		return ret;
>>  	}
>>  
> --
> 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/
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[tip:perf/core] perf probe: Improve error messages in --line option

[tip-bot for Masami Hiramatsu]

Commit-ID:  5ee05b8801892ecc5df44e03429008dfa89aa361
Gitweb:     http://git.kernel.org/tip/5ee05b8801892ecc5df44e03429008dfa89aa361
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 6 Jun 2014 07:14:06 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 10 Jun 2014 10:02:06 -0300

perf probe: Improve error messages in --line option

Improve error messages of 'perf probe --line' mode.

Currently 'perf probe' shows the "Debuginfo analysis failed" message with
an error code when the given symbol is not found:

  -----
  # perf probe -L page_cgroup_init_flatmem
  Debuginfo analysis failed. (-2)
    Error: Failed to show lines.
  -----

But -2 (-ENOENT) means that the given source line or function was not
found. With this patch, 'perf probe' shows the correct error message:

  -----
  # perf probe -L page_cgroup_init_flatmem
  Specified source line is not found.
    Error: Failed to show lines.
  -----

There is also another debug error code is shown in the same function
after get_real_path(). This removes that too.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20140606071406.6788.47850.stgit@kbuild-fedora.novalocal
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 44c7141..9a0a183 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
 
 	ret = debuginfo__find_line_range(dinfo, lr);
 	debuginfo__delete(dinfo);
-	if (ret == 0) {
+	if (ret == 0 || ret == -ENOENT) {
 		pr_warning("Specified source line is not found.\n");
 		return -ENOENT;
 	} else if (ret < 0) {
-		pr_warning("Debuginfo analysis failed. (%d)\n", ret);
+		pr_warning("Debuginfo analysis failed.\n");
 		return ret;
 	}
 
@@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
 	ret = get_real_path(tmp, lr->comp_dir, &lr->path);
 	free(tmp);	/* Free old path */
 	if (ret < 0) {
-		pr_warning("Failed to find source file. (%d)\n", ret);
+		pr_warning("Failed to find source file path.\n");
 		return ret;
 	}
 

Re: [PATCH -tip 0/4] perf/probe: Improve error messages

[Arnaldo Carvalho de Melo]

Em Fri, Jun 06, 2014 at 07:13:38AM +0000, Masami Hiramatsu escreveu:
> Here is a series that improves error messages of perf probe.
> 
>  -  Improve the error message if we can not find given member in
>     the given structure. (perf probe --add)
>  -  Show error code and description only in verbose mode if
>     perf probe command is failed.
>  -  Improve error messages of perf probe --vars mode.
>  -  Improve error messages of perf probe --line mode.
> 
> This series is mainly for fixing confusing messages and
> for removing the error code which is just a mysterious
> number for users.

Thanks, its better now!

Jiri, I'll process these patches,

- Arnaldo